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

imp: Make package name a required argument in link #64

Merged
merged 4 commits into from
Jan 10, 2019
Merged

imp: Make package name a required argument in link #64

merged 4 commits into from
Jan 10, 2019

Conversation

Esemesek
Copy link
Member

@Esemesek Esemesek commented Jan 8, 2019

Make packageName required in link command.
Fixes: #48

@Esemesek Esemesek requested a review from grabbou January 8, 2019 21:46
@thymikee
Copy link
Member

thymikee commented Jan 8, 2019

What about linking assets like fonts? Is this still supported somehow?
Here's example of usage: https://medium.com/react-native-training/react-native-custom-fonts-ccc9aacf9e5e#bcc2
cc @grabbou

@grabbou
Copy link
Member

grabbou commented Jan 9, 2019

Yes, it is still supported.

To be honest, I wonder if we should keep it. Linking only a single dependency but, at the same time, linking all the assets, looks to me like a really badly designed behaviour.

I wonder whether we should:

  • allow running link without a package name that would only link assets from the project itself (like in the document you presented). The downside of this direction is that it's confusing for those who were running react-native link without a name before.
  • allow running link with the project name, e.g react-native link HelloWorld where HelloWorld is the name inside package.json of the project. That would link assets only, since native files are already linked.

@ferrannp
Copy link
Member

ferrannp commented Jan 9, 2019

Definitely linking assets is needed.

Linking only a single dependency but, at the same time, linking all the assets

That's weird yes, unless the single dependency has assets.

I think something explicit is better, just like:

react-native link --assets

@thymikee
Copy link
Member

thymikee commented Jan 9, 2019

Or linking assets (or whatever there is in the config file) would be performed when no name is passed.

@grabbou
Copy link
Member

grabbou commented Jan 9, 2019

Or linking assets (or whatever there is in the config file) would be performed when no name is passed.

Problem with this is that it's an implicit breaking change. People running react-native link and not being aware of this change, will report problems of native files not being linked.

Since we are forcing the name, I guess the better approach is to require the name of the project itself or... create another command (probably unwanted).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants