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

RN's package.json specifies in-exact version numbers, which routinely breaks builds. #12772

Closed
MattFoley opened this issue Mar 7, 2017 · 17 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@MattFoley
Copy link

Description

The package.json for this project specifies almost all dependencies with a ^. This routinely breaks older builds.

React Native 0.32.1 was previously using uglify-js 2.8.5. This morning/last night uglify-js published v2.8.8. Since RN specifies "uglify-js": "^2.6.2" in it's package.json, npm began pulling in the latest v2.8.8.

This new version of uglify-js now conflicts with lodash 4.1.x usage alongside react-native. Our build which worked just fine yesterday was broken by this change. runInContext was no longer defined correctly on a release/minified build from the react-native packager. See npm page here for release history of uglify-js.

Comparison of symbol definition change causing this breakage:

uglifyjs@2.8.5:

e.runInContext=n

uglifyjs@2.8.8:

r.runInContext=runInContext

This change meant that our JS file threw an exception on load, and was completely broken. This has to be the dozenth time this has happened to since switching to React Native, and has wasted countless hours.

Solution

Stop using in-exact versions. Specify exactly the version of dependency you need. (Why on earth would you want dependency version changes happening underneath you?)

Additional Information

  • React Native version: 0.32.1
  • Platform: iOS/Android
  • Operating System: OSX
@ide
Copy link
Contributor

ide commented Mar 7, 2017

Using exact versions in package.json doesn't solve this thoroughly because of transitive dependencies. Use yarn.lock on the app developer's side if you want to defend against this.

@brentvatne
Copy link
Collaborator

brentvatne commented Mar 7, 2017

Thanks for catching this @MattFoley. I agree that using yarn.lock would help you avoid this problem on your app, although what happens for new projects? The template for init doesn't include yarn.lock and so all new projects will break. For people still using npm there isn't much they can do either (I guess theres shrinkwrap, but..)

I was able to fix this on someone's project by running yarn add uglify-js@2.8.5 -save. @ide I think we could also tighten up the ^ to ~ to reduce the odds of this happening.

@MattFoley
Copy link
Author

MattFoley commented Mar 7, 2017 via email

@MattFoley
Copy link
Author

MattFoley commented Mar 7, 2017 via email

@MattFoley
Copy link
Author

MattFoley commented Mar 7, 2017 via email

@duhseekoh
Copy link

Just re-bundled and can confirm the change in the uglify repo fixed the issue. I was experiencing this issue on RN 0.36.1 today.

@ide
Copy link
Contributor

ide commented Mar 8, 2017

Nice effort put in by @brentvatne to help with the debugging to get a patch out. Going to close this issue since we think it's been resolved upstream. If you're running into this problem, clear your node_modules (or yarn upgrade) and try installing them fresh again.

@ide ide closed this as completed Mar 8, 2017
@kyle-ssg
Copy link

kyle-ssg commented Mar 8, 2017

I gather this is why I'm getting Unhandled JS Exception: Can't find variable: runInContext running a release build. However yarn add uglify-js@2.8.5 -save didn't seem to solve this. Is this issue unrelated?

edit: Yes it was unrelated, the js file was silently failing to minify due to an incorrect and unused js import. If anyone with the same issue spots this I'd recommend manually bundling JS order to debug these sorts of issues with react-native bundle --platform ios --dev false --entry-file index.ios.js --bundle-output iOS/main.jsbundle

@MattFoley
Copy link
Author

MattFoley commented Mar 8, 2017 via email

@duhseekoh
Copy link

Uglify 2.8.9 (published a few hours ago) the latest works. It was only 2.8.8 that broke RN. Shouldn't need to lock in at 2.8.5 as was previously suggested.

@kyle-ssg
Copy link

kyle-ssg commented Mar 8, 2017

Ah right, I think in my case it was an unfortunate combination of the two issues in that case - I'd already got 2.8.5 before diagnosing my import issue. Glad this is sorted anyway!

@brentvatne
Copy link
Collaborator

Resolved in mishoo/UglifyJS#1573

@dslounge
Copy link

dslounge commented Mar 9, 2017

Oh man, the last two days have suucked because of this. Eat your vegetables and shrinkwrap your dependencies kids.

@kevinhylant
Copy link

kevinhylant commented Mar 9, 2017

Thank you for figuring this out! @MattFoley
I had literally just upgraded React-Native & React so I had re-yarn.lock-ed (bad timing, I guess)

@petermikitsh
Copy link

Well, that was an hour of my life I won't get back. Glad I found this issue. This is impacting newer versions as well. I'm on RN 0.41.2 and npm i --save uglify-js@2.8.9 fixed the problem.

@MattFoley
Copy link
Author

@brentvatne / @ide I believe I'm running into this same problem again just now, and I'm digging into what the specific issue is.

I'm looking at an old version of my code base that hasn't changed in months, and when rebuilding now, a ListView in that version no longer works properly, and I'm seeing a slew of dependency differences from the npm install. These are all coming from automated build jobs, so user error is doubtful. Will re-ping this thread if I solve it, currently looking at uglifyjs again, which was on v2.7.5 last this was built, and is now on v2.8.12.

@MattFoley
Copy link
Author

That looks like that was the problem again. @brentvatne Thank you for adding that to the package.json! (v2.7.5 was exactly the right version to be on it looks like!)

facebook-github-bot pushed a commit that referenced this issue Mar 22, 2017
Summary:
As per uglify-js maintainer kzc's comment in mishoo/UglifyJS#1573 (comment) we should be locking our version to prevent issues like #12772 from happening again.

No test plan needed, people are already using this version of uglify-js (it's the latest).
Closes #12802

Differential Revision: D4749853

Pulled By: javache

fbshipit-source-id: 866a19cb2c1add31b55e14d0f4dadb7f68fda64c
rkretzschmar added a commit to rkretzschmar/react-native that referenced this issue Apr 5, 2017
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

9 participants