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

Presence of local-cli __fixtures__ file breaks react-native-vector-icons #17610

Closed
dave-irvine opened this issue Jan 15, 2018 · 14 comments
Closed
Labels
Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.

Comments

@dave-irvine
Copy link

dave-irvine commented Jan 15, 2018

Is this a bug report?

yes

Have you read the Contributing Guidelines?

yes

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 6.9.2
Yarn: Not Found
npm: 5.6.0
Watchman: 4.7.0
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: 2.3 AI-162.4069837

Packages: (wanted => installed)
react: 16.2.0 => 16.2.0
react-native: 0.52.0 => 0.52.0

Steps to Reproduce

  1. Install https://github.com/oblador/react-native-vector-icons
  2. Use in a component
  3. Start app

Expected Behavior

App starts successfully

Actual Behavior

Error is shown:

error: bundling failed: Error: While resolving module `react-native-vector-icons/MaterialIcons`, the Haste package `react-native-vector-icons` was found. However the module `MaterialIcons` could not be found within the package. Indeed, none of these files exist:

* `/Users/user/my-app/node_modules/react-native/local-cli/core/__fixtures__/files/MaterialIcons(.native||.ios.js|.native.js|.js|.ios.json|.native.json|.json)`
* `/Users/user/my-app/node_modules/react-native/local-cli/core/__fixtures__/files/MaterialIcons/index(.native||.ios.js|.native.js|.js|.ios.json|.native.json|.json)`

Steps to fix

As per oblador/react-native-vector-icons#626

rm ./node_modules/react-native/local-cli/core/__fixtures__/files/package.json

It looks like this file is outdated for some time, I'm not sure if it needs to be there but it should either be updated or removed. For some reason it refers directly to the react-native-vector-icons project.

@dave-irvine dave-irvine changed the title Presence of __fixtures__ file breaks react-native-vector-icons Presence of local-cli __fixtures__ file breaks react-native-vector-icons Jan 15, 2018
@vovkasm
Copy link
Contributor

vovkasm commented Jan 16, 2018

Workaround: addition of rn-cli.config.js (for RN 0.52 at least):

const blacklist = require('metro/src/blacklist')
module.exports = {
  getBlacklistRE () {
    return blacklist([/react-native\/local-cli\/core\/__fixtures__.*/])
  },
}

It seems some blacklist items were lost in the process of moving from RN to metro :-/

@ide ide added Help Wanted :octocat: Issues ideal for external contributors. Tooling labels Jan 16, 2018
@afilp
Copy link

afilp commented Jan 16, 2018

@vovkasm Thanks, where do we add this file? At the root of the app?

@vovkasm
Copy link
Contributor

vovkasm commented Jan 16, 2018

@afilp rn-cli.config.js should be located in the root directory of the project. It is location where RN will find it automatically (#7271). Seems this info not documented.

@CityLifeCorp
Copy link

Try this
rm ./node_modules/react-native/local-cli/core/fixtures/files/package.json

@afilp
Copy link

afilp commented Jan 18, 2018

@CityLifeCorp Thanks, I think though that we should not do this because next time we 'npm i' (or a colleague does this on her computer) the change will be lost. At least rn-cli.config.js will be committed to GIT, so everyone will have it when working with the project and no further action will be needed.

This is why I believe a 'permanent' change needs to be made in 'react-native' logic itself and be released in a next version.

t4deu added a commit to t4deu/react-native that referenced this issue Jan 19, 2018
Include a default blacklist in the build settings to prevent
processing of incorrect fixture files by Metro.
@t4deu
Copy link
Contributor

t4deu commented Jan 19, 2018

Hello guys, just sent a PR with a fix, hope it helps everyone

t4deu added a commit to t4deu/react-native that referenced this issue Jan 19, 2018
Include a default blacklist in the build settings to prevent
processing of incorrect fixture files by Metro.
t4deu added a commit to t4deu/react-native that referenced this issue Jan 19, 2018
Include a default blacklist in the build settings to prevent
processing of incorrect fixture files by Metro.
@dazweeja
Copy link

dazweeja commented Jan 25, 2018

I don't think that package.json from version 1.0.0 of react-native-vector-icons needs be in the react-native repo at all. As far I can see, there were references to the com.oblador.vectoricons.VectorIconsPackage in local-cli/link/fixtures/android/0.17/patchedMainActivity.java and local-cli/link/fixtures/android/0.18/patchedMainActivity.java but these are outdated/no longer required?

Edit: Updated my comment after further research.

@vovkasm
Copy link
Contributor

vovkasm commented Jan 25, 2018

@dazweeja
If you delete local-cli/core/__fixtures__/files/package.json this test will fail:
local-cli/core/__tests__/findAssets.spec.js.

I think that it was not supposed to be "package.json from old version of react-native-vector-icons", instead it was added as "package.json from some real module to test RN behavior with real-world complicated example". I think it totally valid to have any package.json or other files with any content in repo for tests.

But __fixtures__ directories as collection of data for tests definitely should not be included in module search paths.

@dazweeja
Copy link

@vovkasm
Thanks. That explanation makes sense. The PR from @t4deu seems like a good solution in that case.

@Yandamuri
Copy link

Yandamuri commented Jan 30, 2018

@vovkasm
Your solution worked for me. Thank you. But Could you tell why the issue is accruing? I ma new to react-native

@vovkasm
Copy link
Contributor

vovkasm commented Feb 1, 2018

@Yandamuri Sorry I couldn't. I'am only one of many developers that use RN in their work... But it seems real issue is #17677 and what I know is that issue was introduced between version 0.51.0 and 0.52.0 of RN.

t4deu added a commit to t4deu/react-native that referenced this issue Feb 14, 2018
Include a default blacklist in the build settings to prevent
processing of incorrect fixture files by Metro.
t4deu added a commit to t4deu/react-native that referenced this issue Feb 14, 2018
Include a default blacklist in the build settings to prevent
processing of incorrect fixture files by Metro.
Plo4ox pushed a commit to Plo4ox/react-native that referenced this issue Feb 17, 2018
Summary:
Include a default blacklist into the build settings to prevent
processing of incorrect fixture files by Metro.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Fix facebook#17610 issue, preventing metro from processing fixture files

1. Have a working demo
2. Install https://github.com/oblador/react-native-vector-icons
3. Use in a component
4. Start the app
5. The app starts successfully and display the icons

[ GENERAL  ]  [ BUGFIX ]  [local-cli/util/Config.js] - Add default file blacklist
Closes facebook#17672

Differential Revision: D7014627

Pulled By: hramos

fbshipit-source-id: 20974e6fdd0977eeeb1048c29c9d621c803c26e9
@AylwSt01
Copy link

will this fix be available in 0.54?

@tychota
Copy link
Contributor

tychota commented Feb 21, 2018

Would like to backport this commit to 0.53-stable as well !

cc @grabbou

@grabbou
Copy link
Contributor

grabbou commented Feb 21, 2018

Oh, yeah, it's unfortunate. I'll cherry-pick that onto both branches and let it wait for a while before next release to make sure I catch all the commits.

grabbou pushed a commit that referenced this issue Feb 21, 2018
Summary:
Include a default blacklist into the build settings to prevent
processing of incorrect fixture files by Metro.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Fix #17610 issue, preventing metro from processing fixture files

1. Have a working demo
2. Install https://github.com/oblador/react-native-vector-icons
3. Use in a component
4. Start the app
5. The app starts successfully and display the icons

[ GENERAL  ]  [ BUGFIX ]  [local-cli/util/Config.js] - Add default file blacklist
Closes #17672

Differential Revision: D7014627

Pulled By: hramos

fbshipit-source-id: 20974e6fdd0977eeeb1048c29c9d621c803c26e9
grabbou pushed a commit that referenced this issue Feb 21, 2018
Summary:
Include a default blacklist into the build settings to prevent
processing of incorrect fixture files by Metro.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Fix #17610 issue, preventing metro from processing fixture files

1. Have a working demo
2. Install https://github.com/oblador/react-native-vector-icons
3. Use in a component
4. Start the app
5. The app starts successfully and display the icons

[ GENERAL  ]  [ BUGFIX ]  [local-cli/util/Config.js] - Add default file blacklist
Closes #17672

Differential Revision: D7014627

Pulled By: hramos

fbshipit-source-id: 20974e6fdd0977eeeb1048c29c9d621c803c26e9
fcangialosi pushed a commit to inizio-inc/react-native that referenced this issue Jul 18, 2018
Summary:
Include a default blacklist into the build settings to prevent
processing of incorrect fixture files by Metro.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Fix facebook#17610 issue, preventing metro from processing fixture files

1. Have a working demo
2. Install https://github.com/oblador/react-native-vector-icons
3. Use in a component
4. Start the app
5. The app starts successfully and display the icons

[ GENERAL  ]  [ BUGFIX ]  [local-cli/util/Config.js] - Add default file blacklist
Closes facebook#17672

Differential Revision: D7014627

Pulled By: hramos

fbshipit-source-id: 20974e6fdd0977eeeb1048c29c9d621c803c26e9
@facebook facebook locked as resolved and limited conversation to collaborators Feb 17, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.