oblador / react-native-vector-icons

Customizable Icons for React Native with support for image source and full styling.
https://oblador.github.io/react-native-vector-icons/
MIT License
17.39k stars 2.12k forks source link

Error loading package.json in fa5-upgrade script #800

Closed willcosgrove closed 6 years ago

willcosgrove commented 6 years ago

I've got a brand new RN app created from Ignite. I upgraded react-native-vector-icons to 5.0.0, and ran the fa5-upgrade script. It looks like it worked, but there is an error in the output:

➜  my_app git:(master) ✗ ./node_modules/.bin/fa5-upgrade
Setting up npm config
Please enter your FontAwesome5 npm token:
********-****-****-****-************
Creating temporary folder
Created folder /var/folders/j7/yp0hmwd91tj03yj0sby4ky540000gn/T/rnvi.c3Y5sKG7
/var/folders/j7/yp0hmwd91tj03yj0sby4ky540000gn/T/rnvi.c3Y5sKG7 ~/Code/my_app
Downloading FontAwesome5 Pro
~/Code/my_app
Copying font files
Modifying package.json
module.js:549
    throw err;
    ^

Error: Cannot find module '../../package.json'
    at Function.Module._resolveFilename (module.js:547:15)
    at Function.Module._load (module.js:474:25)
    at Module.require (module.js:596:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/Users/will/Code/my_app/node_modules/react-native-vector-icons/bin/add-font-assets.js:6:14)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
Linking project
Scanning folders for symlinks in /Users/will/Code/my_app/node_modules (26ms)
rnpm-install info Platform 'ios' module react-native-vector-icons is already linked
rnpm-install info Platform 'android' module react-native-vector-icons is already linked
rnpm-install info Linking assets to ios project
rnpm-install info Linking assets to android project
rnpm-install info Assets have been successfully linked to your project
Done

a couple minutes of debugging elapses

The icons were not working, I got an error complaining that the font couldn't be found. Investigating the error above, I think I found out what didn't work.

I think this line: https://github.com/oblador/react-native-vector-icons/blob/5384b96b4617e545f6636868eb8945a268a9d068/bin/add-font-assets.js#L6 Needs to be

const json = require('../../../package.json');

../../ lands you in the node_modules folder, ../../../ lands you in the project folder, which will have the package.json

Because this script didn't run correctly, my package.json never got the assets added to it, and so the font didn't link up in my build.

I'm happy to open a PR to fix this if you concur that this is the proper fix.

willcosgrove commented 6 years ago

@hampustagerud, Joel told me that you might be the best person to talk to about this

oblador commented 6 years ago

Hmm, think it should be const json = require(path.resolve('./package.json'));.

willcosgrove commented 6 years ago

Oh nice! Yeah that looks better 😄

hampustagerud commented 6 years ago

The path is wrong yes, it should be ../../../package.json.

Not sure how path.resolve works, does it work its way backwards in the path? If so, wouldn't it pick the path to the package.json from react-native-vector-icons (i.e. ./node_modules/react-native-vector-icons/package.json) before reaching the project folder (which contains the file that should be modified)? I'm not that familiar with the NodeJS API so I'm not really sure.

I ran with:

const json = require(path.join(process.cwd(), 'package.json'));

which worked (supposing the script is ran from the root of the project which the guide suggests).

oblador commented 6 years ago

@hampustagerud path.resolve uses cwd so it's essentially the same thing. Do you have a branch/PR for this or should I create one?

hampustagerud commented 6 years ago

I keep forgetting that only require is relative to the current file...

I can get a PR together and submit it later tonight when I get home!