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

Filter published files on npm #5626

Merged
merged 3 commits into from Jul 18, 2017
Merged

Filter published files on npm #5626

merged 3 commits into from Jul 18, 2017

Conversation

TrySound
Copy link
Contributor

For now not gzipped leaflet package is 23mb. There's weird 4mb file .goutputstream-N6HL8X and a lot other stuff which increases node_modules size. files field can fix this with specified whitelist.

For now not gzipped leaflet package is 23mb. There's weird 4mb file `.goutputstream-N6HL8X` and a lot other stuff which increases node_modules size. `files` field can fix this with specified whitelist.
@perliedman
Copy link
Member

Hi and thanks! This does indeed look like a very good idea.

A question to understand this change: with this change, we include src although not strictly necessary (the main attribute is set to dist/leaflet-src.js). Is this to make it possible to import specific parts of Leaflet as modules, or is the intention to still supply a somewhat complete version of Leaflet? If the later, would it also make sense to include spec?

I'm not too familiar with how npm packages are normally distributed, so trying to understand what would be best for us here.

@TrySound
Copy link
Contributor Author

Yeah. Source is useful to reduce bundle size, classes are not treeshakable by bundlers. I don't see spec necessary in published package.

Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

This looks good! But given that we already have an .npmignore file, it would be nice to have this in one place — i.e. either in files or npmignore but not both, to make it easier to maintain.

@TrySound
Copy link
Contributor Author

Sure, forgot to remove :)

@mourner mourner merged commit b343cf7 into Leaflet:master Jul 18, 2017
@TrySound TrySound deleted the patch-1 branch July 18, 2017 15:11
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