Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more files to npm distribution #2552

Merged
merged 3 commits into from
Nov 17, 2018
Merged

Conversation

lukastaegert
Copy link
Member

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:

Description

At the moment, both the browser esm build as well as the generated sourcemaps are not published to npm. This will change the configuration to include those files as well.

Copy link
Contributor

@shellscape shellscape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't appear to have any negative consequences. Does it solve a particular issue?

@lukastaegert
Copy link
Member Author

Mainly that we have been building an ESM browser build for quite some time but forgot to publish it. And I guess the source maps that had been created but were not delivered do not hurt as well.

@guybedford
Copy link
Contributor

And I guess the source maps that had been created but were not delivered do not hurt as well.

It seems like the source maps will increase the package download size by around 4MB, which may not be ideal I think?

Also I'm not sure the debugging experience is necessarily improved by having the source maps included? It's pretty standard for built libraries to not bundle their source maps. If we turned them off entirely that would even give us faster builds?

@lukastaegert
Copy link
Member Author

Good point regarding performance but I think the main time-sink is typescript and minification of the browser bundle at the moment. And I guess finding error locations in tests will be more difficult? I will play around with this a little.

I did not check the size of the source maps. Yes, more than doubling the size of our generated package is probably not ideal. Will change this to only add the browser build.

@lukastaegert
Copy link
Member Author

Ok

  • removed sourcemaps from bundle
  • only generate sourcemaps where they actually help us in tests
  • switch the browser extension from mjs to es.js. I originally preferred mjs but I feel it is just very inconsistent (and no breaking change since it was not part of the bundle orignally)
  • use patterns in file property so that we do not miss files in the future

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.

None yet

3 participants