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

Transpilation of V2.0.0+ #10565

Closed
arindam1993 opened this issue Apr 13, 2021 · 36 comments
Closed

Transpilation of V2.0.0+ #10565

arindam1993 opened this issue Apr 13, 2021 · 36 comments
Assignees

Comments

@arindam1993
Copy link
Contributor

arindam1993 commented Apr 13, 2021

With GL-JS v2 we switched the main bundle to ES6 from ES5 to unlock the various benefits of allowing ES6 features into the public API, but this tripped up a lot of existing build systems as evident in #10173.
We understand having to change your build configuration can be frustrating, but we feel the tradeoff for doing this once is definitely worth it. Here are some raw performance stats:

  • The size of the mapbox-gl.js bundle goes down by 66KB ( 827 KB with ES6 vs 893 KB with ES5)
  • Load times improve by up to 6%
  • Heavily CPU bound code paths like map.queryRenderedFeatures() can be up to 20% faster. (Querying for fill-extrusions at high pitch is 40% faster)

Apart from raw numerical performance wins, there are a bunch of subjective benefits too:

  • We kept our transpilation pretty light and didn’t heavily polyfill. This prevented us from using newer JS API features like Promises, but now we can start designing API’s around modern JS features.
  • Opens the door for even more optimizations in the future, for example, currently we’re just a single bundle, but ES6 supports dynamic imports. We could feasibly figure out a way to load the library in chunks, further improving time to first render!

We want to help make this transition as easy as possible! We recently updated our documentation so that it goes into much more detail on how a build system can be set up in various ways to cleanly transpile GL-JS. You should also start seeing an error message that is very explicit when bundle parsing fails, and provides a link to the documentation.

create-react-app implementation:

One of the project environments people have already outlined in #10173 as having issues with is using create-react-app. Here’s what you can do in that situation:

  • Update the production browserslist key in package.json to the ones recommended here. This is what it would look like when applied to the default config generated by [email protected]
"browserslist": {
  "production": [
    ">0.2%",
    "not dead",
    "not op_mini all",
    "not chrome < 51",
    "not safari < 10"
  ],
  "development": [
    "last 1 chrome version",
    "last 1 firefox version",
    "last 1 safari version"
  ]
}

Make sure to update the usage database in caniuse-lite to ensure that these settings apply correctly.
h/t to @zacharyliu for this solution

OR

Angular v11 implementation:

H/t to @OleksandrYatsiuk for this one

We’ve probably not covered every possible way a Javascript project can be set up in our documentation, so we wanted to use this ticket to discuss those situations. If the provided solutions don’t work for you or if you find places for improvement, please let us know below!

@ocommaj
Copy link

ocommaj commented May 12, 2021

Just pushed a commit that fixed this transpile error in one of my repos (after several commits that did not) --

In my case I needed to use the craco solution from @mtuanp in a project bundled using Create-React-App with the react-map-gl wrapper and built/deployed via DigitalOcean static site

@mikelmaron mikelmaron unpinned this issue May 12, 2021
@arindam1993 arindam1993 pinned this issue May 17, 2021
@cigzigwon
Copy link

whelp. Just FYI. Nothing from this thread or real works with Gatsby. i've even overriden the webpack config to exclude the mapbox-gl dir as it is used by another dep I don't maintain.

@cigzigwon
Copy link

cigzigwon commented May 19, 2021

This should work and behave like the recommended browserlists solution in the context of babel-preset-gatsby but DOES NOT.
{ "plugins": [ ["@babel/plugin-proposal-decorators", { "legacy": true }] ], "presets": [ [ "babel-preset-gatsby", { "targets": { "browsers": [">0.2%", "not dead", "not ie 11", "not chrome < 51", "not safari < 10"], "node": 6 } } ] ]

@cigzigwon
Copy link

cigzigwon commented May 19, 2021

And my last suggestion... If I'm skipping transpiling in the dir then why still WebWorker bundle error??

/**
 * Implement Gatsby's Node APIs in this file.
 *
 * See: https://www.gatsbyjs.com/docs/node-apis/
 */

// You can delete this file if you're not using it
exports.onCreateWebpackConfig = ({ actions, loaders, getConfig }) => {
  const config = getConfig()

  config.module.rules = [
    // Omit the default rule where test === '\.jsx?$'
    ...config.module.rules.filter(
      rule => String(rule.test) !== String(/\.jsx?$/)
    ),

    // Recreate it with custom exclude filter
    {
      // Called without any arguments, `loaders.js()` will return an
      // object like:
      // {
      //   options: undefined,
      //   loader: '/path/to/node_modules/gatsby/dist/utils/babel-loader.js',
      // }
      // Unless you're replacing Babel with a different transpiler, you probably
      // want this so that Gatsby will apply its required Babel
      // presets/plugins.  This will also merge in your configuration from
      // `babel.config.js`.
      ...loaders.js(),

      test: /\.jsx?$/,

      // Exclude all node_modules from transpilation, except for 'swiper' and 'dom7'
      exclude: modulePath =>
        /node_modules\/mapbox-gl/.test(modulePath),
    },
  ]

  // This will completely replace the webpack config with the modified object.
  actions.replaceWebpackConfig(config)
}

@cigzigwon
Copy link

Here's the problem in Gatsby. Has nothing to do with transpiling at all. It all works as expected. Gatsby has issues with libs when a window is undefined. So thus there is nothing being added to the dynamic workerURL where the bundled script should be hosted. So it doesn't matter what dist I use minified or not because that url will never be set and Gatsby's solution to use a null loader does not work so it looks like you would absolutely have to change the above node script to annex a new rule to use a null loader. i've tried that but then things just end up blowing up on build. This was a recommendation for using the 1.12 mapbox-gl impl. Am I missing something? I don't really understand how your implementation works.

var shared, worker, mapboxgl;
// define gets called three times: one for each chunk. we rely on the order
// they're imported to know which is which
function define(_, chunk) {
if (!shared) {
    shared = chunk;
} else if (!worker) {
    worker = chunk;
} else {
    var workerBundleString = "self.onerror = function() { console.error('An error occurred while parsing the WebWorker bundle. This is most likely due to improper transpilation by Babel; please see https://docs.mapbox.com/mapbox-gl-js/api/#transpiling-v2'); }; var sharedChunk = {}; (" + shared + ")(sharedChunk); (" + worker + ")(sharedChunk); self.onerror = null;"

    var sharedChunk = {};
    shared(sharedChunk);
    mapboxgl = chunk(sharedChunk);
    if (typeof window !== 'undefined') {
        mapboxgl.workerUrl = window.URL.createObjectURL(new Blob([workerBundleString], { type: 'text/javascript' }));
    }
}
}

@cigzigwon
Copy link

I reverted to 1.13.0 and it works! You DO NOT need a null loader implementation becasue my build works as expected. I don't know which version of Gatsby had the problem but its old news.

You need to change the way your dist is compiled because it doesn't look to ever export the WebWorker bundle

@rreusser
Copy link
Contributor

rreusser commented May 19, 2021

@cigzigwon Sorry it's been challenging to get this working. I was able to get a basic gatsby example working by excluding mapbox-gl from transpilation by excluding mapbox-gl from transpilation with:

import mapboxgl from '!mapbox-gl'

It sounds like this may not match the details of your usage though. In particular, maybe you were using a third party wrapper which prevented this setup from working?

Any extra info about your particular requirements (e.g. is transpilation required? Which of the tradeoffs in the solutions in the docs are acceptable?) would be very much appreciated so we can solve the issues and help communicate the best solutions for different environments.

@cigzigwon
Copy link

cigzigwon commented May 20, 2021

@rreusser I'm throwing a new project together that is somewhat useful but the project itself will add to the KB of migrating a core application over to Gatsby. So I'm on a hinge. The old stuff uses Webpack bundler with 1.13.0 and the new stuff will be Gatsby. I was directed to the Docs but the problem for me is trying to leverage react-mapbox-gl which will work with mapbox-gl v2 and React 17. People seem to struggle w/async to get that to work but will indeed work. So in theory that library would still be a problem since it doesn't use '!mapbox-gl'. I do appreciate the advice but it is really just a transparency problem w/Gatsby for me as opposed to Webpack where I know a bit more about what is going on. I do like the getConfig() func in Gatsby... but it's like... what am I really getting?? and I'm being advised to override in this way via Gatsby. see here

What I will do is branch off on the older project that uses Webpack v4 and mapbox-gl v1 because we will need v2 there ASAP as we have alot going on in our maps! And then I'll do more research/learning with Gatsby but we are going to toss out react-mapbox-gl JSX lib because our custom useMapbox React hook has 80% of base functionality for any React app. The react-mapbox-gl has other issues on the horizon anyhow when dealing with events and should only really be used w/MB-GL JS v1.13.0 anyhow. I would also advise others to spin up Gatsby and move off CRACO because it's not very good for scalability anyhow.

@cigzigwon
Copy link

@arindam1993 The problem is that we opt in to things like Netlify and Gatsby to save time and money. I don't really want to be hacking into that and other outdated dependencies for these fixes. I don't think the browserslist fix is a viable solution for most because it's not going to discount the in-lining code in this way. I don't understand how this way of calling is ever going to yield consistent behavior there. Is that transpilation being plastered into a polyfill hook factory module? I don't really understand how that works and yields diff results in the console. It looks to be non-sequential. I just don't understand it. I think the worker-loader lib is going to be mandatory.

@DanielJackson-Oslo
Copy link

Has anyone gotten this to work with Next.js? Struggling here :)

@DanielJackson-Oslo
Copy link

Has anyone gotten this to work with Next.js? Struggling here :)

In the end I managed to get it working in Next.js by exchanging mapbox-gl with maplibre-gl. I'm still not sure why downgrading would not work, but at least this did! 🎉

I'm using react-map-gl so I haad to do it in a bit of a roundabout way:

Here is how:

Install maplibre-gl

Then add an alias replacing all instances of mapbox-gl with maplibre-gl in webpack:

My next.config.js:

module.exports = {
    webpack: (config) => {
        config.module.rules.push({
            resolve:{
                alias: {
                    ...config.resolve.alias,
                    'mapbox-gl': 'maplibre-gl'
                }
            }
        })
      // Important: return the modified config
      return config
    },
  }

@arthur-clifford
Copy link

I finally got around to upgrading to mapbox-gl 2.2 in my angular project.

!mapboxgl itself worked but was still throwing an error in my IDE saying that it couldnt find any types for mapbox-gl

For me this worked:
// @ts-ignore
import mapboxgl from '!mapbox-gl';

I know that "ignoring " errors isn't normally recommended but I think in this case it was related to something in my stack not understanding what the ! meant while the builder knew not to compile mapbox-gl per the mapbox documentation.

With ts-ignore my app compiles and builds without error and without stalling on any errors and without configuring any special builders.

I Hope this helps somebody.

@cigzigwon
Copy link

@arthur-clifford It would help if you shared what transpilation method you are using.... otherwise it just seems related to your IDE and doesn't belong here.

@cigzigwon
Copy link

@DanielJackson-Oslo That library is a fork of v1 anyhow so doesn't help anybody here. Also... why make the alias? Why not update the requires so people later on know that it's not mapbox-gl when they work on a view using it.

@cigzigwon
Copy link

@rreusser I'm not a webpack guru but in Gatsby I'm going to load the worker-loader directly like indicated.
import Worker from "worker-loader!./Worker.js";
One might incur that the bang there is also helpful to avoid the transpilation of the file being loaded. Inlining in this way would be acceptable. This in theory should get us someplace. I just finished switching to our custom hook and weeding out v1 dependants. I'll let you know how things go!

@arthur-clifford
Copy link

arthur-clifford commented May 28, 2021 via email

@cigzigwon
Copy link

We just ended up using the worker-loader plugin in Gatsby. We also ignored the warning for a missing Webpack dependency. We load the plugin as indicated in the Mapbox v2 docs under v2 Transpiling. We found we shouldn't really be editing the webpack config on the fly as indicated in the gatsby docs because there will be more going on there than just simply using Webpack 4 projects as indicated. "Lazy loading" seems to just be more of a headache than using the worker-loader in this way. Cheers!

@cigzigwon
Copy link

@arthur-clifford Okay. I don't know alot about TS but I think it would probably be best to only compile specific typeroots instead of everything under the sun. That way you won't have TS looming in on everything. It's not really a specific solution for your case but probably a better practice. Less files to enumerate? Perhaps? TS Config typeRoots

justinanderson added a commit to CxSci/atlas-of-opportunity that referenced this issue Jun 22, 2021
Mapbox GL JS is in a weird state with their 2.x release and doesn't play nicely with things that can't support ES6 yet. See mapbox/mapbox-gl-js#10565 for a deeper explanation.
mkysft added a commit to mkysft/superformula-fullstack that referenced this issue Oct 1, 2021
- Avoid improper transpilation by babel.
- See mapbox/mapbox-gl-js#10565
AliceR added a commit to NASA-IMPACT/admg-casei that referenced this issue Oct 5, 2021
@amcc
Copy link

amcc commented Dec 11, 2021

With this version: "mapbox-gl": "^2.6.1", I can simply import mapbox like this in Gatsby v 3.12

import mapboxgl from "!mapbox-gl";

No other configuration is needed in gatsby-node.js and the map displays on build without errors

@openSourcerer9000
Copy link

openSourcerer9000 commented Jan 20, 2022

Aughhhhhhhhh

Fix 1 doesn't work for me, craco doesn't look like it supports CRA v5 (which I used), what's Fix #3?

@JoeDuncko
Copy link

JoeDuncko commented Feb 16, 2022

Updating the production browserslist key in package.json doesn't seem to work anymore. Do we know why? I assume it's not related to CRA v5.

EDIT: Just noticed @zacharyliu's answer above . That seemed to work for me. Unfortunately, I agree that that's pretty flaky... how can we make it less flakey, and make sure the docs are updated with such?

@jonesmac
Copy link

Same as @JoeDuncko. I'm using CRA v5 and @zacharyliu's answer worked and I also believe its pretty flaky. I think the good news is that the percentage of unsupported browsers for this case will decrease overtime so the 0.2% should be ok. If anyone from mapbox is monitoring this thread, the post at the top really needs to be changed as well as the docs with the and supports es6-class update.

@edmondklai
Copy link

I am using CRA v5, I ejected the react app to get access to webpack.config.js. I added these two lines and it seems to work for me.

            {
              test: /\.(js|mjs|jsx|ts|tsx)$/,
              include: paths.appSrc,
              loader: require.resolve("babel-loader"),
              options: {
                ignore: ["./node_modules/mapbox-gl/dist/mapbox-gl.js"],
              ....
            }
            {
              test: /\.(js|mjs)$/,
              exclude: /@babel(?:\/|\\{1,2})runtime/,
              loader: require.resolve("babel-loader"),
              options: {
                ignore: ["./node_modules/mapbox-gl/dist/mapbox-gl.js"],
              ....
            }

@abrman
Copy link

abrman commented Apr 23, 2022

I'm not sure if this is the current best solution but I dropped support for android < 5 & internet explorer all together because of their partial support of ES6 (https://caniuse.com/es6)
Here's my browserlist that seems to work

"browserslist": {
    "production": [
      ">0.2%",
      "not dead",
      "not op_mini all",
      "not safari < 10",
      "not chrome < 51",
      "not android < 5",
      "not ie < 12"
    ],
    "development": [
      "last 1 chrome version",
      "last 1 firefox version",
      "last 1 safari version"
    ]
  },

kandl added a commit to kandl/gooddata-create-gooddata-react-app that referenced this issue Jun 17, 2022
- Otherwise, GeoCharts are not working in boilerplate due to some babel transforms
- See related MapBox issue: mapbox/mapbox-gl-js#10565
- https://docs.mapbox.com/mapbox-gl-js/guides/install/#transpiling

JIRA: RAIL-4312
kandl added a commit to kandl/gooddata-create-gooddata-react-app that referenced this issue Jun 17, 2022
- Otherwise, GeoCharts are not working in boilerplate due to some babel transforms
- See related MapBox issue: mapbox/mapbox-gl-js#10565
- https://docs.mapbox.com/mapbox-gl-js/guides/install/#transpiling

JIRA: RAIL-4312
kandl added a commit to kandl/gooddata-create-gooddata-react-app that referenced this issue Jun 17, 2022
- Otherwise, GeoCharts are not working in boilerplate due to some babel transforms
- See related MapBox issue: mapbox/mapbox-gl-js#10565
- https://docs.mapbox.com/mapbox-gl-js/guides/install/#transpiling

JIRA: RAIL-4312
kandl added a commit to kandl/gooddata-ui that referenced this issue Jun 17, 2022
- Otherwise, GeoCharts are not working in boilerplate due to some babel transforms
- See related MapBox issue: mapbox/mapbox-gl-js#10565
- https://docs.mapbox.com/mapbox-gl-js/guides/install/#transpiling
- Also add step to create-react-app article

JIRA: RAIL-4312
kandl added a commit to kandl/gooddata-ui that referenced this issue Jun 17, 2022
- Otherwise, GeoCharts are not working in boilerplate due to some babel transforms
- See related MapBox issue: mapbox/mapbox-gl-js#10565
- https://docs.mapbox.com/mapbox-gl-js/guides/install/#transpiling
- Also add step to create-react-app article

JIRA: RAIL-4312
@cigzigwon
Copy link

cigzigwon commented Jun 20, 2022

In Gatsby load the worker w/worker-loader Webpack plugin:

// in ./gatsby-node.js
exports.onCreateWebpackConfig = ({ actions: { replaceWebpackConfig }, getConfig }) => {
  const config = getConfig()

  // for loading node_modules/mapbox-gl/dist/mapbox-gl-csp-worker.js
  config.module.rules.push({
    test: /csp-worker\.js$/,
    use: { loader: "worker-loader" },
  })

  config.output.chunkLoadingGlobal = 'this'

  replaceWebpackConfig(config)
}

@Countdown369
Copy link
Contributor

This issue is now closed courtesy of @abrman 's solution. We have updated https://docs.mapbox.com/mapbox-gl-js/guides/install/#targeting-transpilation-to-es6-with-browserslist .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests