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

fix: Remove unnecessary files from npm package #3054

Merged
merged 3 commits into from
Jul 8, 2021

Conversation

stephenmathieson
Copy link
Member

We just had to manually publish @next, as a develop build failed. When doing this, I noticed how many things are included in the tarball we send to npm. This patch drastically reduces the size of the tarball and the number of files it contains.

Previously:

npm notice === Tarball Details ===
npm notice name:          axe-core
npm notice package size:  811.9 kB
npm notice unpacked size: 3.5 MB
npm notice total files:   689

Now:

npm notice === Tarball Details ===
npm notice name:          axe-core
npm notice package size:  314.3 kB
npm notice unpacked size: 1.5 MB
npm notice total files:   7

This will hopefully reduce the chance of another failed upload.

We just had to manually publish `@next`, as [a `develop` build failed](https://app.circleci.com/pipelines/github/dequelabs/axe-core/2462/workflows/19d5c98e-c7e4-4a85-87f1-3503179ac326/jobs/33275). When doing this, I noticed how many things are included in the tarball we send to npm. This patch drastically reduces the size of the tarball and the number of files it contains.

Previously:

```
npm notice === Tarball Details ===
npm notice name:          axe-core
npm notice package size:  811.9 kB
npm notice unpacked size: 3.5 MB
npm notice total files:   689
```

Now:

```
npm notice === Tarball Details ===
npm notice name:          axe-core
npm notice package size:  314.3 kB
npm notice unpacked size: 1.5 MB
npm notice total files:   7
```

This will hopefully reduce the chance of another failed upload.
@stephenmathieson stephenmathieson requested a review from a team as a code owner June 29, 2021 18:48
straker
straker previously approved these changes Jul 6, 2021
Copy link
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

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

I believe these are all the files we need, but @WilcoFiers should also approve so we don't mess it up.

Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Not sure why this is better over updating npmignore. At the very least let's not use both.

"axe.js",
"axe.min.js",
"axe.d.ts",
"locales/"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"locales/"
"LICENSE",
"locales/"

Copy link
Member Author

Choose a reason for hiding this comment

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

npm includes this itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it? OK sure cool.

@stephenmathieson
Copy link
Member Author

Not sure why this is better over updating npmignore. At the very least let's not use both.

Good catch. I'll remove the .npmignore. IMO being explicit about what's being published is much better than implicitly including everything other than what's listed in that file.

@stephenmathieson stephenmathieson dismissed WilcoFiers’s stale review July 7, 2021 19:04

Suggested change is redundant; removed .npmignore

"axe.js",
"axe.min.js",
"axe.d.ts",
"locales/"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it? OK sure cool.

@WilcoFiers WilcoFiers changed the title build: Remove unnecessary files from npm package fix: Remove unnecessary files from npm package Jul 8, 2021
@stephenmathieson stephenmathieson merged commit 7600b7c into develop Jul 8, 2021
@stephenmathieson stephenmathieson deleted the smaller-tarball branch July 8, 2021 11:41
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