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

String.split misbehaves in chrome 51 with canary build #2199

Closed
bdwain opened this issue May 17, 2017 · 15 comments · Fixed by #2200
Closed

String.split misbehaves in chrome 51 with canary build #2199

bdwain opened this issue May 17, 2017 · 15 comments · Fixed by #2200
Milestone

Comments

@bdwain
Copy link

bdwain commented May 17, 2017

Can you reproduce the problem with latest npm?

yes

Description

The native string split method seems to no longer work correctly in the console (or my app) when i load my app in production mode (it's fine if i don't minify it). I've tracked this issue down to a specific change in how webpack uses uglify. i filed an uglify issue with a little more detail, but it's still relevant here because it's using those tools.

Expected behavior

In the console:

let str = 'fooBar'
str.split(/(?=[A-Z])/)

> ["foo", "Bar"]

Actual behavior

let str = 'fooBar'
str.split(/(?=[A-Z])/)

> ["fooBar"]

Environment

[email protected]
node: v7.9.0
npm: 4.2.0
yarn (which i'm actually using, not npm): 0.23.3

os: os x sierra
browser: chrome 51 (other versions (back to chrome 30) seem fine)

Reproducible Demo

https://github.com/bdwain/uglify-bug

all you need to do to reproduce is run create-react-app with the canary build of react-scripts, and then npm install (or yarn add) babel polyfill and include it in your entry point. The result of yarn build will then cause the issue. Open a production build of the app (dev mode is fine), then open a console and try to run string split.

@gaearon
Copy link
Contributor

gaearon commented May 17, 2017

We need to check if manually disabling reduce_vars option fixes it.

@bdwain
Copy link
Author

bdwain commented May 17, 2017

@gaearon i went into the react scripts node_modules folder and changed the uglify webpack plugin to look like this

 new webpack.optimize.UglifyJsPlugin({
  compress: {
    warnings: false,
  },
  output: {
    comments: false,
  },
  sourceMap: true,
  reduce_vars: false //added this
}),

but it still failed.

it happens in my app also (which is not using CRA) no matter what options I try to specify

@gaearon
Copy link
Contributor

gaearon commented May 17, 2017

It should be inside compress, can you try that?

@gaearon gaearon added this to the 1.0.0 milestone May 17, 2017
@bdwain
Copy link
Author

bdwain commented May 17, 2017

ah that fixes it! good catch. is that a bug in that uglify feature then?

@gaearon
Copy link
Contributor

gaearon commented May 17, 2017

Yes.

@bdwain
Copy link
Author

bdwain commented May 17, 2017

cool thanks for your help!

@gaearon
Copy link
Contributor

gaearon commented May 17, 2017

Can you confirm my fix works for you?
#2200

@bdwain
Copy link
Author

bdwain commented May 17, 2017

@gaearon is there a way i can install that specific build of react-scripts? it looks like the same fix that worked for me above, but i'm unable to run it to make sure.

it seems like react-scripts isn't its own repo so i couldn't have package.json reference the commit hash.

@gaearon
Copy link
Contributor

gaearon commented May 17, 2017

You could replace webpack.config.prod.js with this version to verify.

@bdwain
Copy link
Author

bdwain commented May 17, 2017

oh sorry. then yes it works. that's what i did before when i tested. i thought you wanted me to just run your version by installing it without manually changing it.

@gaearon
Copy link
Contributor

gaearon commented May 17, 2017

Going to keep this closed since we have a workaround but if you see the original issue closed please ping us so we can try reenabling. Thanks for the report!

@bdwain
Copy link
Author

bdwain commented May 17, 2017

will do! thanks for solving this one!

@bdwain
Copy link
Author

bdwain commented May 19, 2017

@gaearon uglify 2.8.27 was released with the fix for this. Not sure if it's we need webpack to upgrade to point their yarn.lock file to the new version also though.

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

Yarn lock doesn't work across packages. But I would feel safer if they released a version bump where they would require at least uglify-js@^2.8.27 in their package.json. Can you send a PR to them for this?

@bdwain
Copy link
Author

bdwain commented May 19, 2017

yup.

@lock lock bot locked and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants