Skip to content

feat(build): Drop prepack step & directly pack /build directory#12656

Merged
mydea merged 14 commits intodevelopfrom
fn/no-more-prepack
Jun 27, 2024
Merged

feat(build): Drop prepack step & directly pack /build directory#12656
mydea merged 14 commits intodevelopfrom
fn/no-more-prepack

Conversation

@mydea
Copy link
Copy Markdown
Member

@mydea mydea commented Jun 26, 2024 •
edited
Loading

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 the package.json.

image

Fixes #12642

There are some tiny fluctuations in the comparison table but as far as I can tell nothing serious/bad.

@mydea mydea self-assigned this Jun 26, 2024
@mydea mydea requested review from a team as code owners June 26, 2024 09:28
@mydea
Copy link
Copy Markdown
Member Author

mydea commented Jun 26, 2024

cc @nicohrubec we should also do this for the nest package right away!

nicohrubec
nicohrubec reviewed Jun 26, 2024
View reviewed changes
packages/svelte/package.json Outdated
"types",
"types-ts3.8"
"/build",
"LICENCE",
Copy link
Copy Markdown
Member

@nicohrubec nicohrubec Jun 26, 2024

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?

Copy link
Copy Markdown
Member Author

@mydea mydea Jun 26, 2024

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!

Copy link
Copy Markdown
Contributor

@lforst lforst Jun 26, 2024

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

Copy link
Copy Markdown
Member Author

@mydea mydea Jun 26, 2024

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

Copy link
Copy Markdown

@alexandersorokin alexandersorokin Jun 26, 2024

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.

andreiborza
andreiborza reviewed Jun 26, 2024
View reviewed changes
packages/angular/package.json Outdated
@@ -14,8 +14,8 @@
"/build/fesm2015",
"/build/fesm2020",
"/build/*.d.ts",
Copy link
Copy Markdown
Member

@andreiborza andreiborza Jun 26, 2024

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?

Copy link
Copy Markdown
Member Author

@mydea mydea Jun 26, 2024

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!

andreiborza
andreiborza approved these changes Jun 26, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@andreiborza andreiborza left a comment

Choose a reason for hiding this comment

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

nice reducing complexity!

AbhiPrasad
AbhiPrasad reviewed Jun 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@AbhiPrasad AbhiPrasad left a 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?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 26, 2024 •
edited
Loading

size-limit report 📦

Path Size
@sentry/browser 22.22 KB (0%)
@sentry/browser (incl. Tracing) 33.31 KB (0%)
@sentry/browser (incl. Tracing, Replay) 69.09 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.42 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) 73.15 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) 85.75 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback, metrics) 87.61 KB (0%)
@sentry/browser (incl. metrics) 26.5 KB (0%)
@sentry/browser (incl. Feedback) 38.86 KB (0%)
@sentry/browser (incl. sendFeedback) 26.85 KB (0%)
@sentry/browser (incl. FeedbackAsync) 31.45 KB (0%)
@sentry/react 24.97 KB (0%)
@sentry/react (incl. Tracing) 36.36 KB (0%)
@sentry/vue 26.33 KB (0%)
@sentry/vue (incl. Tracing) 35.17 KB (0%)
@sentry/svelte 22.36 KB (0%)
CDN Bundle 23.42 KB (0%)
CDN Bundle (incl. Tracing) 35.05 KB (0%)
CDN Bundle (incl. Tracing, Replay) 69.18 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) 74.38 KB (0%)
CDN Bundle - uncompressed 68.8 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 103.66 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed 214.13 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 226.84 KB (0%)
@sentry/nextjs (client) 36.24 KB (0%)
@sentry/sveltekit (client) 33.95 KB (0%)
@sentry/node 132.43 KB (0%)
@sentry/node - without tracing 91.87 KB (+0.01% 🔺)
@sentry/aws-serverless 118.45 KB (+0.01% 🔺)

@mydea
Copy link
Copy Markdown
Member Author

mydea commented Jun 26, 2024

Can we remove/update https://github.com/getsentry/sentry-javascript/blob/develop/scripts/prepack.ts?

That is removed in this PR! 😅

@mydea
Copy link
Copy Markdown
Member Author

mydea commented Jun 26, 2024

For reference, in Angular we still pack from ./build, but that is the way to go there - the angular build command does some magic itself for this.

AbhiPrasad
AbhiPrasad approved these changes Jun 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@AbhiPrasad AbhiPrasad left a 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

@mydea mydea force-pushed the fn/no-more-prepack branch from cda71bf to dba98d7 Compare June 27, 2024 07:33
@mydea mydea merged commit 2a7c22b into develop Jun 27, 2024
@mydea mydea deleted the fn/no-more-prepack branch June 27, 2024 08:57
@AbhiPrasad AbhiPrasad mentioned this pull request Jun 28, 2024
Merged
AbhiPrasad added a commit that referenced this pull request Jul 2, 2024
@AbhiPrasad
fix(deno): Add prepack for deno build (#12700)
840dd8f 
fixes #12698

Adds back prepack script just for deno which was removed in
#12656 because it's a
breaking change for the deno package which relies on the directory
structure for their import path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nicohrubec nicohrubec nicohrubec left review comments

@andreiborza andreiborza andreiborza approved these changes

@lforst lforst Awaiting requested review from lforst

+2 more reviewers

@alexandersorokin alexandersorokin alexandersorokin left review comments

@AbhiPrasad AbhiPrasad AbhiPrasad approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mydea mydea

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

npm packages @sentry/core and @sentry/utils have invalid 'sources' path in *.js.map files

6 participants

@mydea @lforst @alexandersorokin @AbhiPrasad @nicohrubec @andreiborza