maplibre-gl-js icon indicating copy to clipboard operation
maplibre-gl-js copied to clipboard

ECMAScript Modules

Open birkskyum opened this issue 1 year ago • 15 comments

ESM is the new standard with import/export etc. We don't use it now because there are some old mapbox dependencies that only export .cjs, but nothing more than we can handle. Also, firefox has for 5 years struggled to support this in web workers, and now it's about time to move on:

Firefox (Update: Resolved from v114)

Web worker support ships with Firefox 114 PR, Release notes

The issue was around lack ESM support for web workers: https://caniuse.com/mdn-api_worker_worker_ecmascript_modules 5+ years of ongoing effort: https://bugzilla.mozilla.org/show_bug.cgi?id=1247687

Benchmarks

I once attempted to move the benchmark to ESM #964 , but found the change wasn't backward compatible with the already uploaded benchmarks of prev versions, so it was rolled back #969 .

Point of this issue

This can be a tracking issue where we first map the dependencies we have that aren't compatible, and then keep this updated when they are updated

birkskyum avatar Aug 31 '22 11:08 birkskyum

Do you mean that we currently export maplibre as commonjs and we should export it as esm? I think the problem of switching out of commonjs besides the FF support was the ability to have the worker URL pushed while building this. There are some complicated build steps to facilitate for this which I think are very hard to split. I've looked at this when I tried to understand if we could leverage tree shaking here: #977 I generally think this is a good idea, even better if we can introduce it as part of 3.0 version as a breaking change. But I'm not sure how to solve all the build complexity with the workers url and also the shared code between the main thread and the worker thread...

HarelM avatar Aug 31 '22 12:08 HarelM

Yes, export maplibre as esm. Sure, I think we might be able to take small steps towards it. A major blocker previously was i.e. also all this node assert that would break the dev build if exported as esm. The first step is likely to have the build as esm.

birkskyum avatar Aug 31 '22 12:08 birkskyum

The published dist/ dir already has multiple versions: https://unpkg.com/browse/[email protected]/dist/ And indeed, the individual src files appear use import/export, although "bare".

So shouldn't it be easy to just add a new one: maplibre-gl-esm.js? And add a "module": "dist/maplibre-gl-esm.js" to package.json? That seems to be the common approach for folks wanting to support both forms.

For now, skypack works:

        import maplibreGl from 'https://cdn.skypack.dev/maplibre-gl'

.. but we'd like to avoid skypack's conversions which sometimes fail. And it would be nice to use other cdn's as well.

backspaces avatar Sep 04 '22 21:09 backspaces

import maplibreGl from 'https://cdn.skypack.dev/maplibre-gl'

@backspaces That doesn't work for me:

var map = new maplibreGl.Map()

gives me an [ERR_UNSUPPORTED_ESM_URL_SCHEME] error.

geraldo avatar Sep 19 '22 15:09 geraldo

Try it using this:

<!DOCTYPE html>
<html>
<head>
    <title>Imports</title>
</head>
<body>
    <script type="module">
        import maplibregl from 'https://cdn.skypack.dev/maplibre-gl'
        console.log('https://cdn.skypack.dev/maplibre-gl', maplibregl)
    </script>
</body>
</html>

backspaces avatar Sep 20 '22 15:09 backspaces

Thanks, that works fine when run directly in the browser, and even shows a basic map:

var map = new maplibregl.Map({
	container: 'map',
	style: 'https://demotiles.maplibre.org/style.json',
	center: [0, 0],
	zoom: 0
})

But when I try to run it with node (using this https module loader), new maplibregl.Map does throw an error:

ReferenceError: devicePixelRatio is not defined

geraldo avatar Sep 21 '22 07:09 geraldo

The devicePixelRatio likely fail because there is no device/display. What do you hope to achieve running it in node ? Depending on your use-case, you might want to use the node distribution of maplibre-gl-native instead https://www.npmjs.com/package/@maplibre/maplibre-gl-native

birkskyum avatar Sep 21 '22 07:09 birkskyum

The use case would be including Maplibre as dependency, for example offering a maplibre renderer in Maputnik or other kind of web apps depending on Maplibre. So it still would be web and not Android nor iOS, that's why I think using maplibre-gl-native would not work right?

geraldo avatar Sep 22 '22 14:09 geraldo

If it's rendering the vector map in the browser, gl-js is the place to go. If it's server-rendering, the gl-native can be used.

birkskyum avatar Sep 22 '22 15:09 birkskyum

Related to https://github.com/maplibre/maplibre/discussions/159 through tree-shaking

birkskyum avatar Jun 12 '23 09:06 birkskyum

I have made a lot of change in version 4 and basically removed the global object that existed before. It might be interesting to try out again to export the library in both esm and common js.

HarelM avatar Mar 06 '24 09:03 HarelM

The article which I'm looking at right now is the following: https://www.sensedeep.com/blog/posts/2021/how-to-create-single-source-npm-module.html

Since we add a package.json file to dist folder in order to force bundling tools to treat maplibre as common js I think we'll need to place the esm output in a folder called mjs or something to avoid it being treated as commonjs.

I'll do some experiments locally and see if I can get to a working version or not. Since this is not a replacement and only an addition there's no breaking change (hopefully) and I'll be able to push it to a regular version and not a breaking change version. later on we can decide if and when to deprecated commonjs build output.

HarelM avatar May 10 '24 11:05 HarelM

Seems to work now as recent Maputnik versions (v2.0) use MapLibre 4.1.2 as dependency.

geraldo avatar May 10 '24 12:05 geraldo

Below is a link to an article on esm with rollup and worker configuration: https://justinribeiro.com/chronicle/2020/07/17/building-module-web-workers-for-cross-browser-compatibility-with-rollup/ This is to overcome the amd usage for the code to load it to the worker. The above suggest to export the worker code as text - which means no reuse of shared code between the worker and the main thread. If there's no code sharing then the csp approach is probably better. Seems like esm and loading the worker code automatically are contradicting one another. So the current build configuration is the best solution as far as I understand from bundle size perspective. We might consider introducing esm to the csp build, but this will have very small impact as I believe most users are not using the csp build.

If anyone has more information on this or knows this topic more than I do please feel free to let me know.

HarelM avatar May 12 '24 10:05 HarelM

@timocov sorry to tag and feel free to unsubscribe, but it would be great if you can take a peak at this and let me know if I'm missing anything - I know you have a lot of knowledge/experience around rollup, build, esm, commonjs amd etc, and any input would be helpful here.

HarelM avatar May 12 '24 13:05 HarelM