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

Webworkify + Uglify-js + Require.js not getting along #29

Closed
etpinard opened this issue Aug 17, 2016 · 8 comments
Closed

Webworkify + Uglify-js + Require.js not getting along #29

etpinard opened this issue Aug 17, 2016 · 8 comments
Labels

Comments

@etpinard
Copy link

Hi, I'm trying to load up a browserify --standalone bundle that includes webworkify that been minified with uglifyjs using the --mangle and --compress options in Require.js.

Problem

Require.js fails to load the bundle:

image

Reproducible example

See https://gist.github.com/etpinard/8ec2c29210d5adee049d54bb427bbbe6

Observations

This line seems to be causing the issue when compressed by uglifyjs

Replacing

'var f = require(' + stringify(wkey) + ');' +
'(f.default ? f.default : f)(self);'

by

'var f = require('+stringify(wkey)+');' +
'(f.default ? f.default : f)(self);'

and running browserify and uglifyjs without the --compress option is enough to replicate the problem listed above.

etpinard added a commit to plotly/plotly.py that referenced this issue Aug 17, 2016
- browserify/webworkify#29
- offline was broken in plotly.py 1.12.6 where
  plotly.js was updated to 1.16.4
etpinard added a commit to plotly/plotly.py that referenced this issue Aug 17, 2016
- browserify/webworkify#29
- offline was broken in plotly.py 1.12.6 where
  plotly.js was updated to 1.16.4
@mourner mourner added the bug label Sep 7, 2016
@mourner
Copy link
Collaborator

mourner commented Sep 7, 2016

I'm really puzzled by this. Why would adding spaces around + sings in webworkify code affect things in any way?

@etpinard
Copy link
Author

etpinard commented Sep 7, 2016

This one is definitively strange. It may be a bug with Require.js itself?

etpinard added a commit to plotly/plotly.js that referenced this issue Sep 7, 2016
@mourner
Copy link
Collaborator

mourner commented Sep 8, 2016

@etpinard would you mind checking if the same thing happens in #31? Just in case.

@etpinard
Copy link
Author

etpinard commented Sep 8, 2016

I get the same Require.js error using off mourner:no-eval , unfortunately.

@etpinard
Copy link
Author

etpinard commented Sep 8, 2016

This commit 31c27ef appear to fix this issue.

@mourner
Copy link
Collaborator

mourner commented Sep 8, 2016

@etpinard but why?

@mourner
Copy link
Collaborator

mourner commented Oct 6, 2016

Since this seems like a require+uglify bug, should we close this issue and wait for upstream fixes?

@etpinard
Copy link
Author

etpinard commented Oct 6, 2016

Since this seems like a require+uglify bug

I believe this is a Require.js bug, but I haven't had the time to investigate further.

For our needs, I was able to fix this issue using a dirty string .replace here and test it here. So you can close this issue if you like.

Thanks for your time.

@mourner mourner closed this as completed Oct 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants