Skip to content
This repository has been archived by the owner on Sep 20, 2019. It is now read-only.

Reduces the package output #929

Merged
merged 1 commit into from
Jun 16, 2018
Merged

Reduces the package output #929

merged 1 commit into from
Jun 16, 2018

Conversation

abdonrd
Copy link
Contributor

@abdonrd abdonrd commented May 8, 2018

Fixes #928.

This reduces the package size.

@web-padawan
Copy link
Contributor

Consider using "files" property instead to specify the included files explicitly: https://docs.npmjs.com/files/package.json#files

@abdonrd
Copy link
Contributor Author

abdonrd commented May 8, 2018

@web-padawan should be like this, right?

  "files": [
    "bundles/**/*",
    "entrypoints/**/*",
    "custom-elements-es5-adapter.js",
    "webcomponents-bundle.js",
    "webcomponents-bundle.js.map",
    "webcomponents-loader.js"
  ]

Or am I leaving something?

@dfreedm
Copy link
Contributor

dfreedm commented May 8, 2018

Hmm, is package size a concern?

@dfreedm dfreedm self-assigned this May 8, 2018
@abdonrd
Copy link
Contributor Author

abdonrd commented May 8, 2018

@azakus Less is better. And we would save ourselves problems like having to ignore the gulpfile.js in the PSK and in the other templates.

@abdonrd abdonrd changed the title Add .npmignore file Reduces the package output May 8, 2018
@abdonrd abdonrd changed the base branch from v2 to master May 9, 2018 19:10
@abdonrd
Copy link
Contributor Author

abdonrd commented May 9, 2018

Rebased on master and updated PR branch! :)

@keanulee
Copy link
Contributor

+1 on reducing package size. This seems to be what most other npm packages do (request, redux, sw-precache, and more).

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

This seems like a good change to me.

@abdonrd
Copy link
Contributor Author

abdonrd commented May 30, 2018

Thanks for the review, @TimvdLippe!
Hoping it can be merged.

@TimvdLippe
Copy link
Contributor

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants