mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.07k stars 2.21k forks source link

Provide a ESM module build #8676

Open pdesjardins90 opened 5 years ago

pdesjardins90 commented 5 years ago

I'm trying to use the library for the first time and the quickstart tutorial example for basic installation doesn't work as expected.

Importing the library like so:

import mapboxgl from 'mapbox-gl'

Throws the error:

Uncaught SyntaxError: The requested module 'node_modules/mapbox-gl/dist/mapbox-gl.js' does not provide an export named 'default'

mapbox-gl-js version: 1.2.1

browser: Chrome

Steps to Trigger Behavior

  1. yarn add mapbox-gl
  2. In a module: import mapboxgl from 'mapbox-gl'
  3. Just open the page with chrome

Link to Demonstration

Can't provide a JSBin since it needs to be installed as an npm package. I basically just followed your quickstart documentation here with the module bundler option

Expected Behavior

When loading the page, there should be no error in the console and the library could be used normally as stated in the docs.

Actual Behavior

When loading the page, I get the error described above.

mourner commented 5 years ago

@pdesjardins90 can you set up a sample repo or a gist reproducing this? Are you using any bundlers like Webpack / Parcel for the page?

pdesjardins90 commented 5 years ago

I've setup a sample repo ~here~

I'm using es-dev-server to serve the index at localhost:8080 and convert bare module specifiers to relative path specifiers (that the browser can understand)

I've tried writing the import as a relative path in case it had something to do with the import path rewrite but I still got the error

mourner commented 5 years ago

@pdesjardins90 what does it say if you do import * as mapboxgl from 'mapbox-gl'?

pdesjardins90 commented 5 years ago

It's then an empty symbol, if I try to do:

mapboxgl.accessToken = 'some-token'

It will refuse to do so because it will consider it a new property assignment into a non-extensible object

mourner commented 5 years ago

I think the issue here is that "Module bundler" in the docs refers to importing mapbox-gl in JS files that get bundled afterwards (e.g. with Webpack, Rollup, Parcel), not to importing the mapbox-gl build as a module in the browser directly — it's a UMD build, not a module one.

pdesjardins90 commented 5 years ago

Is there any hope you'll offer a way to use native ES module? All the bundlers you mentioned support them

mourner commented 5 years ago

All the bundlers also support UMD. Using ES modules in browsers directly is still rare, but we definitely want to explore that at some point. Related: #6391

pdesjardins90 commented 5 years ago

I agree that using ES modules directly in the browser is still rare, I also understand that UMD builds are supported by bundlers and that the amount of work/gain from such a refactor might not be tempting at all for the moment.

I read around a bit and it seems that the future-proof way of doing things for a library would be to expose native ES modules only and let users decide how they want to bundle and transpile it (if they want to) in their app.

What's your general opinion on that as a library maintainer? Would it work for mapbox specifically?

hunterloftis commented 5 years ago

Just ran into the same issue @pdesjardins90.

I expect that all node modules will eventually adopt esm and leave it up to applications to determine packaging. As a short-term workaround, I cloned mapbox-gl-gs and changed its rollup format to "esm." The resulting (vendored) JS loads successfully into es-dev-server.

backspaces commented 4 years ago

I use this sort of rollup fragment for all my runtime dependencies that have not supplied an es6 module:

    {
        input: 'node_modules/mapbox-gl/dist/mapbox-gl-unminified.js',
        output: {
            file: 'dist/mapbox-gl.esm.js',
            format: 'esm',
        },
        plugins: [commonjs()],
    },

I just tried this and it seems to work .. it least imports without error:

            import mapbox from '../dist/mapbox-gl.esm.js'

Would mapbox mind adding that to their workflow? Tossing in terser solves the minified version:

    {
        input: 'node_modules/mapbox-gl/dist/mapbox-gl-unminified.js',
        output: {
            file: 'dist/mapbox-gl.esm.min.js',
            format: 'esm',
        },
        plugins: [terser(), commonjs()],
    },

Results:

-rw-r--r--@  1 owen  staff   1.5M May 16 16:35 mapbox-gl.esm.js
-rw-r--r--@  1 owen  staff   744K May 16 16:35 mapbox-gl.esm.min.js
backspaces commented 4 years ago

To be complete, here's what you would need.

First, a config file, rollup.mapbox.js:

import commonjs from 'rollup-plugin-commonjs'
import { terser } from 'rollup-plugin-terser'

export default [
    {
        input: 'node_modules/mapbox-gl/dist/mapbox-gl-unminified.js',
        output: {
            file: 'dist/mapbox-gl.esm.js',
            format: 'esm',
        },
        plugins: [commonjs()],
    },
    {
        input: 'node_modules/mapbox-gl/dist/mapbox-gl-unminified.js',
        output: {
            file: 'dist/mapbox-gl.esm.min.js',
            format: 'esm',
        },
        plugins: [terser(), commonjs()],
    },
]

Then run it, possibly in a package.json run script:

rollup -c rollup.mapbox.js

Naturally you'd have to change a few paths, but this is all that's needed! YaY!

backspaces commented 4 years ago

Sorry to be noisy, but I completed my study of mapboxgl, both code and css, as modules.

I took the above and ran it in a demo showing the USA counties which I originally imported mapboxgl via <script> tags; now converted to the rollup->es6 above. Works.

I also made a module installing the css. This may seem silly but our code base avoids having the .html manage dependencies. We once found over 20% of them no longer used!

Everything works fine.

ericclemmons commented 4 years ago

Would https://github.com/mapbox/mapbox-gl-js/issues/8676#issuecomment-529287673 be accepted as a PR?

I found that StencilJS isn't able to successfully parse mapbox.Map as a type due to the ESM problem described above.

comatory commented 3 years ago

Please provide at least the ES module optionally.

backspaces commented 3 years ago

Using skypack works, they perform module transforms for you: import mapboxgl from 'https://cdn.skypack.dev/mapbox-gl' It's worth following snowpack/skypack, they are very creative and perform the esm conversion locally for you.

On Mon, Feb 15, 2021 at 1:49 AM Ondrej Synacek notifications@github.com wrote:

Please provide at least the ES module optionally.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mapbox/mapbox-gl-js/issues/8676#issuecomment-779062764, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA6LTUN2GM6PEQ66524T6DS7DNYDANCNFSM4IOWGZTQ .

comatory commented 3 years ago

On Mon Feb 15, 2021 at 5:33 PM CET, Owen Densmore wrote:

Using skypack works, they perform module transforms for you: import mapboxgl from 'https://cdn.skypack.dev/mapbox-gl' It's worth following snowpack/skypack, they are very creative and perform the esm conversion locally for you.

Very cool, did not know about this. Works really well. I wonder if there's a way to include styles via some build that would include CSS-in-JS, it'd be neat to just do that one import and not adding style tag to <head>.[1]

I'm using ES modules for quick experimentations, not production apps so it's really convenient to circumvent build tools and just use it in browser directly.

[1] I'm aware of this not being the best for performance and also it should be agnostic of the environment where the code is being used so it makes sense that JS and CSS would be separated.

backspaces commented 3 years ago

Actually, there is a skypack issue on this: importing styles. Also data like json. https://github.com/skypackjs/skypack-cdn/issues/107

I currently do this myself two ways

1 - via a general async function. It looks like:

await util.setCssStyle(
    'https://cdn.skypack.dev/mapbox-gl/dist/mapbox-gl.css'
)

Here's the code:

export async function setCssStyle(url) {
    const response = await fetch(url)
    if (!response.ok) throw Error(`Not found: ${url}`)
    const css = await response.text()
    document.head.innerHTML += `<style>${css}</style>`
}

The second way is via a "wrapper" cli which takes the css text and outputs a module which wraps the actual css and installs it in <head> for you. The wrapper works like this:

function processData(data) {
    data = `document.head.innerHTML += \`
<style>
${data}
</style>\``
    return data
}

This will be much simpler when top level async arrives. Chrome's next major release will include it.

But lets hope the github issue is resolved with yet another nifty solution from the skypack team!

beginor commented 3 years ago

Import maps is enabled by default with chromium based browsers, please provide an esm build of mapbox-gl, I think many people like me would like to try a bundle free development.

benatshippabo commented 1 year ago

Can we also get a working official esm build for the cdn as well? It's 2023 and many people would like to just do

import mapboxgl from 'https://api.mapbox.com/mapbox-gl-js/v2.12.0/mapbox-gl.js';
ersk commented 1 year ago

Would also like this functionality.

wibed commented 1 year ago

painfully stubbed my toe. autsch

szulcus commented 1 year ago

Any update? Temporary I use /* eslint-disable import/default, import/no-named-as-default-member */ as workaround

marksteward commented 1 year ago

All the bundlers also support UMD. Using ES modules in browsers directly is still rare, but we definitely want to explore that at some point. Related: #6391

Using ES modules in browsers directly would no longer be considered rare.

Igoryoka commented 2 months ago

+1

yosiasz commented 2 months ago

greetings, where are we at with this esm build. is it a yes or should we move on to leaflet? thanks

navidemad commented 2 months ago

+1

marksteward commented 2 months ago

The +1s aren't very helpful. Try esm.sh as a workaround for now:

<script type="importmap">
{
  "imports": {
    "mapbox-gl": "https://esm.sh/mapbox-gl@3.5.1"
  }
}
</script>
...
<script type="module">
import mapboxgl from 'mapbox-gl';

mapboxgl.accessToken = '...';
const map = new mapboxgl.Map({
  container: 'map',
  center: [0, 51.5],
  zoom: 9
});

</script>