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

Build fails when using mapbox-gl webpack 2 and UglifyJSPlugin #4359

Closed
henningko opened this issue Mar 3, 2017 · 62 comments
Closed

Build fails when using mapbox-gl webpack 2 and UglifyJSPlugin #4359

henningko opened this issue Mar 3, 2017 · 62 comments

Comments

@henningko
Copy link

mapbox-gl-js version:
0.32.1

Steps to Trigger Behavior

Set up Mapbox with webpack 2 via vuejs-template

Configuration is as follows:
Base:
https://gist.github.com/henningko/5a76d12a14e485bfc4dd62c2e50ad08e
Production:
https://gist.github.com/henningko/3387f41a55558af60205ea00bbc6b530

Mapbox is used with import mapboxgl from 'mapbox-gl'.

Expected Behavior

Mapbox can be successfully uglified.

Actual Behavior

When building for production, UglifyJS2 has Mapbox throw a Uncaught ReferenceError: n is not defined.
This error also occurs when using Google Closure instead of UglifyJS.
May be related to #1649 , which mainly treats webpack v1 and is closed.

Thanks for your help!

@stevewillard
Copy link

Seeing this as well. Anyone know which version I can downgrade to, which minifies properly?

@henningko
Copy link
Author

Fixed the problem by setting devtool: 'eval' instead of source-map. Not sure why source-map causes trouble for Mapbox, though.

@mourner
Copy link
Member

mourner commented Mar 3, 2017

Could this be a WebPack 2 bug? This doesn't seem to be a fault on the GL JS side.

@stevewillard
Copy link

@mourner yeah definitely. The weird thing is that this was working about a month ago for me. I'm trying to narrow down which dependency is causing the issue.

@henningko eval works for me too, but what about production? cheap-module-source-map has the same issue

@henningko
Copy link
Author

henningko commented Mar 3, 2017

@stevewillard I was just happy to get a minified version out. 😐 I have opened an issue with webpack, too. It's still a gnarly workaround to get them to function together.

@kachkaev
Copy link

kachkaev commented Mar 13, 2017

Having this in production build since the end of last week, after upgrading mapbox-gl and its react wrapper. Could you please share your workaround guys?

PS: Could not find any mentioning of this error message outside the context of mapbox-gl-js.

@foundryspatial-duncan
Copy link

foundryspatial-duncan commented Mar 13, 2017

For what it's worth, I built an app about a month ago with mapbox-gl 0.32.1 and webpack 2.2.0 and it built correctly. I just started an almost-identical project with mapbox-gl 0.33.0 and webpack 2.2.1 and I'm having the same problem described above. (Although in my case it says Uncaught ReferenceError: e is not defined. instead of n. This configuration does build if I completely disable the UglifyJsPlugin.

I tried moving mapbox-gl to its own un-minified chunk (didn't work, more problems from Webpack), but that seems like it could fix the problem. The "dist" code in the npm package is minified anyway, so I'd like to tell the uglifier to leave it alone...

@foundryspatial-duncan
Copy link

With the version numbers mentioned above, it does build properly if you add the option to not optimize comparisons:

        new webpack.optimize.UglifyJsPlugin({
            sourceMap: true,
            compress: {
                warnings: false,
                comparisons: false,  // don't optimize comparisons
            },
        }),

So, it's something in that Uglify function that's wrecking Mapbox.

@mourner
Copy link
Member

mourner commented Mar 15, 2017

@foundryspatial-duncan thank you for finding the potential source of the issue!

Looks like it should be fixed upstream and there's probably nothing we can do on the GL JS side, so I'm closing the issue for now. If you submit related reports in the relevant repos, please link them here to track resolution progress!

@zezhipeng
Copy link

zezhipeng commented Mar 21, 2017

module: {
     ...
     noParse: /(mapbox-gl)\.js$/,
     ...
}

just add this in webpack, will be ok

@shotor
Copy link

shotor commented Apr 3, 2017

@zezhipeng solution works for me.

Not a 100% sure what it does though. Does this mean mapbox-gl is not minified?

@foundryspatial-duncan
Copy link

foundryspatial-duncan commented Apr 3, 2017

I've been importing MapboxGL not using the default entrypoint, but rather:

import mapboxgl from 'mapbox-gl/dist/mapbox-gl';

It says (said?) to do that somewhere in the documentation. I remember trying noParse with the dist version and I don't think I had any luck with it.

edit: I guess that's not the practice with the most recent versions of mapbox-gl. If you just import from mapbox-gl as mentioned above, then the noParse solution does work :)

@mikkelsjolin
Copy link

mikkelsjolin commented Apr 26, 2017

This also happens when running meteor --production with Meteor version 1.4.4.
https://blog.meteor.com/announcing-meteor-1-4-4-11027f33e3bf

@jfirebaugh
Copy link
Contributor

jfirebaugh commented May 24, 2017

I looked into this a bit more:

  • One of the modules in the Mapbox JS bundle has environment detection code of the form typeof global !== "undefined" ? global : typeof self !== "undefined" ? self : ....
  • With certain webpack setups, all of the following are happening:
    1. The whole mapbox-gl.js module is getting wrapped in something like (function (global) { <mapbox gl source> })(...).
    2. Uglify sees that and says "hey, I can mangle global to e" everywhere it appears.
    3. Uglify sees typeof e !== "undefined" and says "hey, I can minimize that to void 0 !== e".
    4. At runtime webworkify extracts this new source void 0 !== e, but stripped of the function (e) { ... } wrapping context that makes it valid.
    5. Without that context void 0 !== e is a ReferenceError rather than true/false.

This is the type of edge case that makes Webpack a huge hassle for both library maintainers and end users, because it doesn't have a way for individual modules to indicate their bundling requirements, like browserify does. Everyone has to encounter these errors for themselves, and then google around for obscure incantations to add to their hundreds-of-lines-long webpack config. There's no way that we as library authors can package Mapbox GL JS so that it "just works" out of the box with Webpack, and in the end, everybody is frustrated.

TL;DR: yet again I conclude that browserify made the right design decisions and webpack the wrong ones.

@davidascher
Copy link

See facebook/create-react-app#2376 (comment) and mishoo/UglifyJS#2011 (comment) for parallel analysis. In particular, from our digging, it seems to be the code that's going through webworkify which is breaking a static code analysis assumption. In mapbox-gl-js, I think the only occurence is:

const WebWorkify = require('webworkify');

which wraps https://github.com/mapbox/mapbox-gl-js/blob/master/src/source/worker.js, but I haven't figured out what in that code or its imports is problematic.

@davidascher
Copy link

@mourner see in particular facebook/create-react-app#2376 (comment) -- it seems that it's either mapbox-gl-js code which is breaking the assumption uglify makes, or a module imported by mapbox-gl-js. I'm hoping that that means mapbox-gl-js is the place a fix can occur.

joedeveloper added a commit to joedeveloper/minifier-js that referenced this issue Jun 18, 2017
Problematic issue what would cause people to disable minification entirely.
relates issues in other projects
facebook/create-react-app#2379
sleepycat/old_usesthis@523c872
mapbox/mapbox-gl-js#4359
mishoo/UglifyJS#2011 (comment)
@felipearosemena
Copy link

Getting the same issue when using gulp-uglify.

@GLosch
Copy link

GLosch commented Jul 13, 2017

Anybody figure out a solution to this? I tried adding mapbox to noParse as suggested here, but no luck. Still getting Uncaught ReferenceError: e is not defined and a broken map.

Using webpack 2.6.1 (with UglifyJsPlugin) and mapbox-gl 0.38.0

@mourner
Copy link
Member

mourner commented Jul 10, 2018

I'm not sure how I can help on a code level (I've not really written web workers before) but if you need more info for diagnosing the things I'm running into I'm happy to supply that.

@Aendrew setting up the most minimal repo or gist that reproduces this with the latest versions of GL JS and Webpack/Uglify so that we have an up-to-date test case would be very helpful.

@anandthakker
Copy link
Contributor

@mourner I set up a minimal repro w/ 0.45 here: https://gist.github.com/anandthakker/6db8a871ccbbaba8afd7deeb483731ef

@mourner
Copy link
Member

mourner commented Jul 11, 2018

@anandthakker I just used your repro and the map works fine for me. Anything I'm missing?

@aendra-rininsland
Copy link
Contributor

aendra-rininsland commented Jul 11, 2018

@mourner Here's a more minimalist repro that reflects the issue: https://github.com/aendrew/mapbox-gl-js--issue-4359-repro

I don't do anything other than instantiate Mapbox and do a build in production mode.

@mourner
Copy link
Member

mourner commented Jul 12, 2018

A PR with a potential solution here: #6956 — let me know what you think!

@anandthakker
Copy link
Contributor

Ah, whoops -- in my gist, I think it works because of the mangle: false option

@mourner
Copy link
Member

mourner commented Jul 12, 2018

@anandthakker interestingly, it still worked for me with mangle: true. 🤷‍♂️

@jfirebaugh
Copy link
Contributor

Can folks here who have been affected by the issue please try out the fix in #6956 (using a build from master) and confirm that it resolves the issue for them and doesn't cause any new issues? If so we'll include it in 0.47.0 final (due out Wednesday 7/18).

@math0ne
Copy link

math0ne commented Jul 16, 2018

Master did not seem to fix this for me but @chriswhong's solution got me going.

@mourner
Copy link
Member

mourner commented Jul 16, 2018

@math0ne can you provide us with a minimal test case for a master build not working with Webpack?

@aendra-rininsland
Copy link
Contributor

@jfirebaugh @mourner Reiterating my comment from #6956, I tried a build from master just now and it seems to work fine. 👍

@swernerx
Copy link

swernerx commented Aug 2, 2018

Looks good for me as well with mapbox-gl 0.47.0. Congrats!

@Tofandel
Copy link

Tofandel commented Nov 22, 2021

I think a similar issue is back with mapbox v2.6.1 and webpack 5 with terser, though it's weird because on prod it always fails to load but on dev it fails only about 50% of the time

Need to investigate a bit more

The error I'm getting is fc.VectorTile is not a constructor

@monolithed
Copy link

I've the same problem with CRA + mapbox-gl@2.6.1. It's not possible to build for production

@monolithed
Copy link

My workaround:

module.exports = {
    babel: {
        loaderOptions: {
            ignore: ['./node_modules/mapbox-gl/dist/mapbox-gl.js'],
        }
    }
};

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

No branches or pull requests