Fix CSP bundle by not mangling class names there, and add CSP tests #11739
Conversation
This file contains 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
mourner
changed the title
ansis
approved these changes
Apr 12, 2022
avpeery
pushed a commit
that referenced
this pull request
Apr 13, 2022
…11739) * fix CSP bundle by not mangling class names there * add a render tests job that uses the CSP bundle * fix lint
karimnaaji
pushed a commit
that referenced
this pull request
Apr 13, 2022
* add missing roundZoom in CustomSource coveringTiles (#11749) * Fix CSP bundle by not mangling class names there, and add CSP tests (#11739) * fix CSP bundle by not mangling class names there * add a render tests job that uses the CSP bundle * fix lint * update patch version too Co-authored-by: Stepan Kuzmin <[email protected]> Co-authored-by: Volodymyr Agafonkin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #11684, fixes #11686 (regression introduced in #11511). Disables class name mangling when making the CSP bundle. To make generic transfer of class instances between web worker and main thread work, we depend on class names being the same on both sides. However, the CSP bundle config runs the bundling process separately for the main thread and the worker, meaning that mangled names won't align as they are in the default bundle.
Disabling class name mangling only increases the size of both bundled files by ~2KB, and it's not the default bundle but rather one for strict CSP environments only, so it should be fine. We could approach this with a more elaborate fix that uses chunks to avoid this tiny overhead, but it doesn't seem to be worth it.
Also needs a test if possible to make sure this doesn't regress in the future.Added a new render test job that uses the CSP bundle to make sure we never regress on this again.Launch Checklist
mapbox-gl-jschangelog:<changelog>Fix the special bundle for CSP-restricted environments that broke in the previous release.</changelog>