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

Extra Backslash Added in Uglifyjs Output of Regex #1929

Closed
ScottHaney opened this issue May 12, 2017 · 10 comments · Fixed by #1959
Closed

Extra Backslash Added in Uglifyjs Output of Regex #1929

ScottHaney opened this issue May 12, 2017 · 10 comments · Fixed by #1959

Comments

@ScottHaney
Copy link

Summary

When running a certain regex through gulp-uglify on my linux build container an extra backslash is added. A separate build from another machine does not include the extra backslash and so the hashes of the outputted files aren't matching.

I created a docker container that reproduces the issue using gulp-uglify since that is where I first posted this question before being redirected to this repo. The code to build the docker image can be found here and uses gulp-uglify to create the issue although I am also able to create the issue inside the container using uglifyjs after installing uglify-js globally from npm.

Uglify version (uglifyjs -V) : 3.0.4

JavaScript input

function boomBaby(s) { return s.split(/[\\/]/); };

The uglifyjs CLI command executed or minify() options used.

uglifyjs called without options on a file with the input javascript listed above

JavaScript output or error produced.

function boomBaby(s){return s.split(/[\\\/]/)}

@kzc
Copy link
Contributor

kzc commented May 12, 2017

Cannot reproduce:

$ bin/uglifyjs -V
3.0.4

$ cat test.js
function boomBaby(s) { return s.split(/[\\/]/); }

$ bin/uglifyjs test.js
function boomBaby(s){return s.split(/[\\/]/)}

@alexlamsl
Copy link
Collaborator

@kzc that's weird - I can reproduce this:

> type test.js
function boomBaby(s) { return s.split(/[\\/]/); };
> uglify-js test.js
function boomBaby(s){return s.split(/[\\\/]/)}

Obviously I'm on Windows, though I doubt that makes a difference.

@kzc
Copy link
Contributor

kzc commented May 13, 2017

The regexes are equivalent in any event.

It's odd that Windows is different. Are you using Chakra?

@kzc
Copy link
Contributor

kzc commented May 13, 2017

Uglify makes no attempt to format RegExp patterns. It relies on the toString() method of the engine.

@kzc
Copy link
Contributor

kzc commented May 13, 2017

Duplicate of #885

@alexlamsl
Copy link
Collaborator

Just vanilla Node.js:

> nvs use node
PATH = node\7.10.0\x64

> echo function boomBaby(s) { return s.split(/[\\/]/); }; | node bin\uglifyjs
function boomBaby(s){return s.split(/[\\\/]/)}

@alexlamsl
Copy link
Collaborator

Ah ha... 🙈

> nvs use node/6
PATH = node\6.10.3\x64

> echo function boomBaby(s) { return s.split(/[\\/]/); }; | node bin\uglifyjs
function boomBaby(s){return s.split(/[\\/]/)}

@kzc
Copy link
Contributor

kzc commented May 13, 2017

Yet another reason not to use node 7.

Anyway, nothing actionable here. We're not going to make a regex parser and formatter.

@alexlamsl
Copy link
Collaborator

For extra reading: https://github.com/eslint/eslint/blob/master/lib/rules/no-useless-escape.js

But yeah, this isn't uglify-regex so I guess we'll have to leave it as is.

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue May 17, 2017
- remove `options.output.unescape_regexps`
- preserve original pattern whenever possible

fixes mishoo#54
fixes mishoo#1929
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue May 17, 2017
- remove `options.output.unescape_regexps`
- preserve original pattern whenever possible

fixes mishoo#54
fixes mishoo#1929
@ScottHaney
Copy link
Author

Thank you so much for your prompt and detailed help! This fixed my issue!!

alexlamsl added a commit that referenced this issue May 17, 2017
- remove `options.output.unescape_regexps`
- preserve original pattern whenever possible

fixes #54
fixes #1929
alexlamsl pushed a commit that referenced this issue Mar 26, 2018
This relates to #1929, but in the context of mozilla AST input/output.
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 a pull request may close this issue.

3 participants