-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(build): Drop prepack step & directly pack /build
directory
#12656
Conversation
cc @nicohrubec we should also do this for the nest package right away! |
packages/svelte/package.json
Outdated
"types", | ||
"types-ts3.8" | ||
"/build", | ||
"LICENCE", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be LICENSE maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, yes it should! 🤦 good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some files including the license are added by default: https://docs.npmjs.com/cli/v10/configuring-npm/package-json#files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, nice, then I'll just remove this overall
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
README.*
is also included by default.
packages/angular/package.json
Outdated
@@ -14,8 +14,8 @@ | |||
"/build/fesm2015", | |||
"/build/fesm2020", | |||
"/build/*.d.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the *.d.ts.map
files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are none here, actually! But I'll still add this, to be future-safe!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice reducing complexity!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove/update https://github.com/getsentry/sentry-javascript/blob/develop/scripts/prepack.ts?
size-limit report 📦
|
That is removed in this PR! 😅 |
For reference, in Angular we still pack from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is removed in this PR
well that's embarrassing
This is mostly an internal change, but it does change the layout of the published package, which shouldn't affect users, unless they depend on non-public things (which nobody should!).
cda71bf
to
dba98d7
Compare
This is mostly an internal change, but it does change the layout of the published package, which shouldn't affect users, unless they depend on non-public things (which nobody should!).
With this change, we will simply pack the packages as-is without moving any folder relativity. This means we do not need a
prepack
step anymore.Instead, we define which files should be included in the tarball via
files
in thepackage.json
.Fixes #12642
There are some tiny fluctuations in the comparison table but as far as I can tell nothing serious/bad.