Expose a worker override from the ESM entry - #13719
Open
tg-freigmbh wants to merge 1 commit into
Open
Conversation
The default bundle exposes `mapboxgl.workerUrl` and `mapboxgl.workerClass` as
documented accessors. The ESM entry has the counterpart to the first one only,
`setWorkerUrl`; there is no way to hand GL JS a worker the application built
itself. That gap is what makes the ESM entry unusable under some bundlers.
`dist/esm/core.js` spawns its worker with `new Worker(new URL('worker.js',
import.meta.url))`, which relies on the host bundler recognising the pattern and
emitting the worker as its own chunk. Vite and webpack 5 do that for
dependencies. Angular's esbuild-based `@angular/build:application` builder
implements it as a TypeScript transformer, so it only ever runs over the
application's own sources and never over prebuilt JS in node_modules. The
expression then survives verbatim into the output, no worker file is emitted, and
the request resolves against the emitted chunk's URL. Behind an SPA fallback that
answers with index.html and status 200 the worker dies silently: the main thread
keeps running and only tile parsing stops.
`setWorkerUrl` can paper over this if the application copies `dist/esm` out as
static assets, but such copies carry no content hash and go stale on upgrade.
Two shapes are implemented here; happy to drop either one.
`setWorkerClass(klass)` mirrors the existing `workerClass` field that
`web_worker.ts` already honours, so it adds no new concept.
`setWorkerFactory(create)` does the same in a shape that type-checks. An
application that has to construct the worker itself can only return an instance,
and TypeScript cannot express a constructor returning an unrelated object, so
every `workerClass` caller ends up asserting the type — including the workaround
in mapbox#13700:
mapboxgl.workerClass = class {
constructor() { return new Worker(new URL(...), {type: 'module'}); }
} as unknown as new () => Worker;
Precedence in the ESM `createWorker` is factory, then class, then url, then the
existing `import.meta.url` default. If the factory would be welcome on the
default entry as well, that is a `mapboxgl.workerFactory` accessor in
src/index.ts plus the same check in web_worker.ts, and it would remove the
assertion from the mapbox#13700 workaround too.
Refs mapbox#13700, mapbox#13678
|
Hey, @tg-freigmbh 👋 Thanks for your contribution to Mapbox GL JS! Important: This repository does not accept direct merges. All changes go through our internal review process. What happens next:
Please respond to any review comments on this PR. For more details, see CONTRIBUTING.md. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a bandaid regarding the problems when trying to use mapbox + 'new' angular build + zones.
See issues #13684
and also technical details in #13700
UMD already exposed
setWorkerClass, ESM did not.However due to more bundler/typescript complexity this API would require ugly casts for every valid use case.
So this pr also adds a 'factory' method, which is slighly more bundler friendly.