Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't copy font files into "assets" android folder after "react-native link" #89

Closed
punksta opened this issue Jan 23, 2019 · 13 comments
Closed
Labels
🗣 Discussion This label identifies an ongoing discussion on a subject

Comments

@punksta
Copy link

punksta commented Jan 23, 2019

Introduction

react-native link is copping all fonts into assets folder for android.
For ios it is registering font files in ios project file without making files copies.

The Core of It

I don't want to store copies of all fonts in android assets folder.

Discussion points

There are few ways of solving this:

  1. We can make gradle task in node_modules/react-native/react.gradle which will copy font files in "generated/assets" in build time. So fonts will be synced automatically same as images.
    I don't know is something similar possible for ios (registering fonts files in build time, without changing source code files)
  2. to make behavior similar to ios link command could just create text file file with font paths to track them instead of making copies. We can add this file to git. And then gradle task adds only fonts from this file, not all of them.

What do you think?

@kelset
Copy link
Member

kelset commented Jan 23, 2019

👋 there - as stated in the README and the issue template, this repo is for long-form discussions. Yours seems more a precise feature request, so I suggest you use Canny instead.

That said, the CLI is been extracted so may be worth posting this issue on the new react-native-cli repo -> https://github.com/react-native-community/react-native-cli

@kelset kelset closed this as completed Jan 23, 2019
@punksta
Copy link
Author

punksta commented Jan 23, 2019

@kelset ok, thanks.

@grabbou
Copy link
Member

grabbou commented Mar 11, 2019

I am actually wondering whether Canny is a good direction for issues like that - this sounds like a change that affects the workflow of every Android developer and might be worth discussing, just like CocoaPods support. Thoughts?

Either way, this is not really a CLI issue, but a React Native one. The default React Native template (and all React Native apps) are done the way that requires copying assets. CLI just automates that behaviour to make it less annoying. We should continue the discussion in the React Native repository itself.

I didn't know there was a way to change it. @punksta - I like first idea with a Gradle task - it would avoid copying assets over yet provide same benefits.

If you'd be willing to give this a go - I am happy to give you a helping hand and tell where to start.

@kelset
Copy link
Member

kelset commented Mar 11, 2019

uhm ok I see your point, I'll reopen for now.

And yes, we need to align on Canny next week.

@kelset kelset reopened this Mar 11, 2019
@kelset kelset added the 🗣 Discussion This label identifies an ongoing discussion on a subject label Mar 11, 2019
@punksta
Copy link
Author

punksta commented Mar 11, 2019

@grabbou I think it's needed to choose a implementation approach. if we've made a gradle task which copies all resources to build dir in buildtime, we'll get discrepancy with ios behavior. Ios requires linking using link command. I don't know is automatic build-time linking possible for ios. If so, link command could do nothing to resources.

https://github.com/unimonkiez/react-native-asset - this library is creating link-assets-manifest.json file during first linking. It uses the file to track resources.

{
  "migIndex": 1,
  "data": [
    {
      "path": "src/assets/font/Roboto.otf",
      "sha1": "27a6cb3bcdb0342ss3aa352fa0c3ccc04b65ddf0"
    }
  ]
}

We can reuse this approach for android with automatic gradle linking. The link command can just write paths to this file and gradle command can read paths and copy files in build temp folder

So I see next ways for now.

  1. gradle task which copies only resources from json tracking file. Manual linking(xcodeproj and plist modification) for ios
  2. gradle task for all resources, something similar for ios. (idk is it possible or not)
  3. gradle task for all resources, manual linking for ios

@grabbou
Copy link
Member

grabbou commented Mar 11, 2019

@grabbou we'll get discrepancy with ios behavior. Ios requires linking using link command. I don't know is automatic build-time linking possible for ios. If so, link command could do nothing to resources.

It's perfectly fine if link for resources on Android would be a noop. Link just automates manual steps that you'd have to do otherwise and they are obviously different depending on the platform.

@ferrannp
Copy link
Member

@punksta love that approach. Meaning the linking will not be moving files around that need to be committed and so on, right? Just that json file. Makes a lot of sense to me.

@punksta
Copy link
Author

punksta commented Mar 12, 2019

@grabbou alright. That's not difficult to make/extend the gradle tasks in rn core. How we sync this change with react-native-cli? it must stop linking fonts for android when gradle task is done. Is there anything else which requires attention?

@grabbou
Copy link
Member

grabbou commented Mar 12, 2019 via email

@punksta
Copy link
Author

punksta commented Mar 12, 2019

@grabbou I'll send pl soon

@grabbou
Copy link
Member

grabbou commented Mar 22, 2019

CC: @bartolkaruza @Salakar - do you think these changes are something we could incorporate directly into CLI newly created Android files? Or it has to land directly to React?

For context, @punksta, we have recently worked on improving the Android linking experience (called auto linking), here's PR: react-native-community/cli#258 that you can check for more info.

@Salakar
Copy link
Member

Salakar commented Mar 22, 2019

do you think these changes are something we could incorporate directly into CLI newly created Android files? Or it has to land directly to React?

@grabbou This could be in the CLI gradle file yes (though won't cover anyone not using it)

@grabbou
Copy link
Member

grabbou commented Apr 17, 2019

This just landed.

facebook/react-native@f01c4e2

@grabbou grabbou closed this as completed Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗣 Discussion This label identifies an ongoing discussion on a subject
Projects
None yet
Development

No branches or pull requests

5 participants