Skip to content

Conversation

jonkoops
Copy link
Contributor

@jonkoops jonkoops commented Aug 1, 2023

Removes the minification of the ESM and CommonJS bundles for distribution. There is no need to minify these files, as they will be consumed by Node.js and bundlers. Not minifying these files also improves debug-ability of these files, as source maps have to be opt-ed into in some environments.

This also removes the source maps from the package, as these are no longer required when shipping unminified source.

@jonkoops jonkoops requested a review from a team as a code owner August 1, 2023 10:43
@jonkoops jonkoops force-pushed the no-minified-js branch 2 times, most recently from 275edac to b575407 Compare August 1, 2023 16:46
@jonkoops
Copy link
Contributor Author

jonkoops commented Aug 3, 2023

Rebased the PR and fixed some merge conflicts, should be good to go now.

@frederikprijck
Copy link
Member

frederikprijck commented Aug 3, 2023

IIRC, we do minify all our JS bundles, but I see the benefit here. Will discuss this with the team as this might be something we want to align (either keep it minified or avoid minifying all) across all our JS SDKs that get bundled.

@jonkoops
Copy link
Contributor Author

jonkoops commented Aug 3, 2023

For bundles that get included directly in the browser I think that would make sense. For example, the UMD bundle is certainly one I expect to get loaded directly. However for ESM and CommonJS that is unlikely, as these are usually consumed by bundlers or Node.js, which would actually benefit from having things as close to source as possible.

@jonkoops
Copy link
Contributor Author

Closing this PR, as #192 will also handle this.

@jonkoops jonkoops closed this Aug 14, 2023
@jonkoops jonkoops deleted the no-minified-js branch August 14, 2023 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants