mapbox/mapbox-gl-js

Build fails when using mapbox-gl webpack 2 and UglifyJSPlugin

henningko opened this issue Β· 62 comments

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!

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

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

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

@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

@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.

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.

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...

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.

@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!

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

just add this in webpack, will be ok

@zezhipeng solution works for me.

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

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 :)

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

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.

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.

@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.

Getting the same issue when using gulp-uglify.

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

@GLosch I solved the problem by adding the following parameters to my uglify call:

uglify({
    sourceMap: true,
    compress: {
        warnings: false,
        comparisons: false,  // don't optimize comparisons
    },
})

I believe the key part in there is the comparisons flag

@felipearosemena I ended up adding mapbox to noParse and that took care of it.

FWIW I was getting the same errors with uglify-js v3.0.24 and browserify (Uncaught ReferenceError: o is not defined). I was using the following command to build my code (which includes require('mapbox-gl')):

browserify src/index.js -t sheetify -t yo-yoify -t es2020 | uglifyjs -c -m > dist/bundle.js

Adding comparisons=false to the uglify compress options fixed it:

browserify src/index.js -t sheetify -t yo-yoify -t es2020 | uglifyjs -c comparisons=false -m > dist/bundle.js

While adding mapbox-gl to noParse resolves the Uncaught ReferenceError, clustering is broken in production builds. No errors, no warnings, but clusters and markers don't appear at all.

Just in case somebody stumbles upon the same use case:

The noParse solution is not compatible with bundle-loader (as in, with noParse, Webpack won't actually write the chunk). I solved this by adding script-loader in front of bundle-loader:

const loadMapboxSdk = require("bundle-loader?lazy!script-loader!mapbox-gl");

I confirm that #4359 (comment) solution works for mapbox-gl 0.40.1 and webpack 3.6.0.

Just came across the same problem when using it with laravel-mix that makes use of webpack and adding noParse to webpack.mix.js worked just fine.

let mix = require('laravel-mix');

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

@mourner Could you re-open? It seems it isn't issue with upstream but this project. Please see:

mishoo/UglifyJS#2520 (comment)

I've hit this issue on a current project as well, using the noParse workaround until it's fixed.

24dev commented

Hey all,

I could really do with some help on this one. I'm using the above with react-static too - but all I'm getting is 'self is not defined'. I have added the noParse workaround but so far no luck.

Any help would be amazing :(

Hi all, we are trying to add mapbox-gl.js to a gatsby.js static site. It works fine in development, but when running gatsby build we get the self is not defined error. Tried the noParse fix, but it doesn't seem to do anything.

 532 | "use strict";var WebWorkify=_dereq_("webworkify"),window=_dereq_("../window"),workerURL=window.URL.createObjectURL(new WebWorkify(_dereq_("../../source/worker"),{bare:!0}));module.exports=function(){return new window.Worker(workerURL)};
  533 | },{"../../source/worker":121,"../window":265,"webworkify":39}],265:[function(_dereq_,module,exports){
> 534 | "use strict";module.exports=self;
      |              ^
  535 | },{}],266:[function(_dereq_,module,exports){
  536 | "use strict";function compareAreas(e,r){return r.area-e.area}var quickselect=_dereq_("quickselect"),calculateSignedArea=_dereq_("./util").calculateSignedArea;module.exports=function(e,r){var a=e.length;if(a<=1)return[e];for(var t,u,c=[],i=0;i<a;i++){var l=calculateSignedArea(e[i]);0!==l&&(e[i].area=Math.abs(l),void 0===u&&(u=l<0),u===l<0?(t&&c.push(t),t=[e[i]]):t.push(e[i]))}if(t&&c.push(t),r>1)for(var n=0;n<c.length;n++)c[n].length<=r||(quickselect(c[n],r,1,c[n].length-1,compareAreas),c[n]=c[n].slice(0,r));return c};
  537 | },{"./util":285,"quickselect":31}],267:[function(_dereq_,module,exports){


  WebpackError: self is not defined
  
  - mapbox-gl.js:534 Object.265
    ~/mapbox-gl/dist/mapbox-gl.js:534:14
  
  - mapbox-gl.js:1 s
    ~/mapbox-gl/dist/mapbox-gl.js:1:614
  
  - mapbox-gl.js:1 
    ~/mapbox-gl/dist/mapbox-gl.js:1:666
  
  - mapbox-gl.js:530 Object.263../window
    ~/mapbox-gl/dist/mapbox-gl.js:530:14
  
  - mapbox-gl.js:1 s
    ~/mapbox-gl/dist/mapbox-gl.js:1:614
  
  - mapbox-gl.js:1 
    ~/mapbox-gl/dist/mapbox-gl.js:1:666
  
  - mapbox-gl.js:155 Object.76.../package.json
    ~/mapbox-gl/dist/mapbox-gl.js:155:14
  
  - mapbox-gl.js:1 s
    ~/mapbox-gl/dist/mapbox-gl.js:1:614
  
  - mapbox-gl.js:1 e
    ~/mapbox-gl/dist/mapbox-gl.js:1:789
  
  - mapbox-gl.js:1 
    ~/mapbox-gl/dist/mapbox-gl.js:1:810

Oddly enough, I was only seeing this when doing an electron build, but a normal webpack production build was fine.

24dev commented

The way we fixed this was by changing the imports to requires, and then wrapping all of the related vars and requires in an if statement to check if it’s running on the client. So, if (window !== undefined) then require etc.

Hope that helps and you’re not going too crazy, I know we were!

eads commented

Confirming same issue as @chriswhong (hi @chriswhong!) with Gatsby. Currently not sure how to solve. noParse did not work for me in a block like so in my gatsby-node.js file:

exports.modifyWebpackConfig = ({ config, stage }) => {
  // Fix for D3
  config.merge({
    node: { fs: 'empty', child_process: 'empty', pym: 'empty' },
    module: { noParse: /(mapbox-gl)\.js$/, },
  })
  return config
}

Hey, we got this working @eads, let me remember how...

This was a two-part fix:

  1. Add some handling to check if window exists before returning anything from render() that calls MapboxGL component. NYCPlanning/labs-geosearch-docs@a855d22#diff-996d85c5d08acf39cf664dbf481cf6ebR29

  2. Add some custom webpack config in Gatsby to use the null loader for Mapbox GL.
    NYCPlanning/labs-geosearch-docs@8240b7d

The easiest and most optimal way is to use CDN version of mapboxgl.

  1. Include the JavaScript and CSS files in the <head> of your HTML file.
<script src='https://api.tiles.mapbox.com/mapbox-gl-js/v0.44.1/mapbox-gl.js'></script>
<link href='https://api.tiles.mapbox.com/mapbox-gl-js/v0.44.1/mapbox-gl.css' rel='stylesheet' />
  1. Exclude mapbox from the output bundles using webpack externals section (https://webpack.js.org/configuration/externals/)
externals: {
  'mapbox-gl': 'mapboxgl' 
}

with mapbox-gl v 0.42.2 and webpack 3.6.0 I had the same issue. solved it with @foundryspatial-duncan's solution.

@yeedle You're probably better off using the workaround that @zezhipeng described using noParse. It's what I've been doing on my own mapbox-gl projects lately. Tweaking the UglifyJsPlugin settings might have other unintended consequences for your project. It's useful/curious to know that that specific optimization is still what's causing trouble, though!

Why would not parsing mapbox-gl at all be better than just not applying the comparison optimizations?

I'm assuming the mapbox-gl package you get from NPM has been compressed and optimized already, so running it through Uglify again would be a waste of time. Plus you'd be turning that optimization off for your whole project, and you might still want to use it on the rest of your code. (Assuming the bug that affects mapbox-gl doesn't affect any of the code you've written!)

In theory, you could avoid minifying all of your vendor packages because they should be minified already.

Makes sense. Thanks for the tip!

I would argue that the commentary in #4359 (comment) is a cheap excuse. Thousands of libraries do not cause any problems here. I would really like to understand what the elementary problem is to simply deploy a correct NPM package that contains a library simply packaged with e.g. Rollup as CJS and ESM.

The current NPM package contains both "src" and "dist" files and is very confusing overall. I figure it's more essential right now to correctly support Webpack and Rollup without requiring some hacks (e.g. noParse, Uglify options) to config files just to work around specific package issues. The code which throws is actually related to old platforms - probably a better approach would be to add a special handling for these instead of requiring all Webpack users to add these hacks (most often after spending hours trying to fix it - like me.)

Would be probably a good chance of making the whole MapBox API more fine-grained and actually using the ESM concept of cherry-picking imports. Right now we import ~150kb (gzipped) of JS for all features. Quite a bundle for rendering a map... and there is no leaner option available.

I mostly agree with @swernerx. Most of npm "just works" with Webpack, and the lean bundle size of Mapbox is really rather big, which might benefit from the tree-shaking techniques of tooling like Rollup and Webpack. I get that y'all like Browserify, but switching to some tooling that's slightly more modern would probably benefit more than just this issue. I digress...

At runtime webworkify extracts this new source void 0 !== e, but stripped of the function (e) { ... } wrapping context that makes it valid.

With all due respect, it seems like you're blaming Webpack for a shortcoming of Webworkify. Apologies if I misunderstand.

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.

Not to put too fine a point on it, but you're referencing a necessarily-complex config that's used in an app scaffolding context and is heavily commented for instructional purposes β€” at least half the LOC are documentation.

I'm not the only person I know to have issues with how MapboxGL works in conjunction with a bundler (specifically Webpack, which, let's be honest, the vast majority of the React-using world incorporates). Querying a mid-sized industry Slack network I'm a part of (comprised mainly of developers in the journalism world, which is not a huge cohort), at least three other people had run into issues with MapboxGL and had given up to the point of simply consuming it via CDN instead of npm.

Please re-open this issue, it points to build system interoperability issues that haven't necessarily been addressed and may not be resolvable simply by using the noParse Webpack config directed as suggested (FWIW, I've never had to do that with any other npm package), as per this and other comments in this thread.

Thanks for the really good discussion @swernerx and @Aendrew. After doing some reading and digging on the cause of this issue, I do think there is likely something in the worker code in the library that is not playing nice with Webpack (or more specifically, I believe Uglify). I haven't pinned down what the exact cause is yet though.

We certainly do not want the process of using Mapbox as a developer to be a pain point so I think this is a legitimate issue that should be dealt with. That being said, the workers are a pretty foundational area of the library so we obviously have to balance usability concerns with keeping the library performant, stable and mostly backwards compatible. I'm going to re-open this ticket to keep this topic alive with the caveat that there's no timeline on when/if this gets fixed to everyone's satisfaction. In the meantime, I'd recommend the noParse solution since it seems to fit the needs of most users. And of course, we would welcome a PR if anyone wants to take on the challenge.

lean bundle size of Mapbox is really rather big, which might benefit from the tree-shaking techniques of tooling like Rollup and Webpack. I get that y'all like Browserify, but switching to some tooling that's slightly more modern would probably benefit more than just this issue. I digress...

@Aendrew FYI, Mapbox GL JS codebase switched to ES modules + Rollup a few months ago (see #6196), so it's fully tree-shaked. The big bundle size stems from the overall complexity of the vector rendering technology, not from bundling inefficiencies.

That being said, we should re-evaluate the issue, since we no longer use Browserify / Webworkify and the details described in #4359 (comment) may no longer fit.

As far as I can tell @mourner the details that John laid out in his comment are still accurate but it doesn't seem to be a bug downstream of us (in Uglify for example). My guess is that a function or functions being passed to our workers are referencing outside variables which are escaping the wrapping context of Uglify causing the error as per this comment from one of the Uglify maintainers. That whole thread is specifically about the issue with our library and provides some good insight if you haven't seen it yet.

Thanks for reopening. 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. πŸ‘

FWIW, setting noParse made the map load for me without throwing, but custom controls I've built (I extended the GeolocateControl class to throw errors if outside a specific bounding box) disappear in production for reasons I'm still trying to isolate. Might be helpful to note, in recent Webpack versions, even if you remove UglifyJsWebpackPlugin from plugins, if you supply the -p (production) flag it will still try to Uglify it, which really confused me for a second. Doing all other prod-related optimisations while disabling Uglify and removing noParse made everything work for me, so it's definitely something to do with Uglify.

Edit: The custom control issue I allude to above was total PEBKAC and unrelated to this issue, didn't realise I was serving my project from a non-TLS origin, which was resulting in the control being unsupported and thus silently not rendering. See #6923.

@Aendrew FYI, Mapbox GL JS codebase switched to ES modules + Rollup a few months ago (see #6196), so it's fully tree-shaked. The big bundle size stems from the overall complexity of the vector rendering technology, not from bundling inefficiencies.

@mourner Ah, I did not know that! Thanks for clarifying!

As I mentioned here and is also indicated in the Uglify thread that @ryanhamley linked here, I think a fundamental issue here is that we use Function.prototype.toString() and Uglify doesn't support it.

Quoting from the Uglify README:

To allow for better optimizations, the compiler makes various assumptions:
[ ... ]
The code doesn't expect the contents of Function.prototype.toString() or Error.prototype.stack to be anything in particular.

And mishoo/UglifyJS#2011 (comment) indicates that handling this is likely out of scope for Uglify.

Using Function.prototype.toString() is a key part of how we set up our web worker code. For reference, it happens here:

var workerBundleString = 'var sharedChunk = {}; (' + shared + ')(sharedChunk); (' + worker + ')(sharedChunk);'

(shared and worker are functions, so concatenating them with + to a string implicitly means using Function.prototype.toString())

There are two ways we could remove this:

  1. Statically inline the worker code into the final bundle, rather than using Function.prototype.toString() to generate the appropriate code at runtime. This would nearly double the size of the mapbox-gl.js script, by duplicating large portions of the code as a string. I don't think this is an acceptable tradeoff.
  2. Create a separate build of gl-js that loads the worker code as a separate script from a specified URL. If you're bundling gl-js with the rest of your app, note that this would entail having a runtime dependency on a remote script that isn't part of your bundle. See #6058

A note for future participants of this thread: we understand firsthand how frustrating it can be to have one's build derailed by complications that seem to come from one particular dependency. That said, this is a complicated problem; our attempts to diagnose it are not "cheap excuses" or buck-passing. Frankly, I do think is a design problem in Webpack, stemming from its default, unconditional use of Uglify, but even so, if there were a straightforward, sustainable way to work around it in the mapbox-gl-js codebase, I'd be happy to make the change -- or to review a PR that did so.

Another possibility people might try is to use UglifyjsWebpackPlugin's exclude option to exclude mapbox-gl from minification. Haven't verified the fix myself yet, but if double-minificiation is the problem that could help.

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 I just used your repro and the map works fine for me. Anything I'm missing?

@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.

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

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

@anandthakker interestingly, it still worked for me with mangle: true. πŸ€·β€β™‚οΈ

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).

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

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

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

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

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

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

My workaround:

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