-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ref(build): Add build/types
to tarballs and adjust types
entry points
#4824
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
Conversation
- change `types` entry point in `package.json` - adjust `.npmignore` to include `build/types` in tarball - applies to `@sentry/angular` & `@sentry/browser` in this commit
change from `files` pkg.json property to .npmignore
adjust .npmignore for consistency w/ other packages
change .npmignore structure to allow specific dirs instead of all js files (consistency)
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.
Lgtm 👍
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.
Question: Does npm require each package to have its own .npmignore
file, or will it use one from the repo if the package itself doesn't have one? (I ask because almost all of these look identical and I'm wondering if it's possible to use a single copy at the project root instead.)
Never mind - I figured this out on my own!
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.
Just two small suggestions, but otherwise LGTM! Can we also add the standard .npmignore
to the nextjs package, so we can stop shipping our tsconfigs, etc?
Thanks for the review. And yes, we can absolutely do that. I'll add the default |
Nope. The |
Alright, then I'll merge this in. Thanks :) |
size-limit report 📦
|
build/types
to tarballs and adjust types
entry point build/types
to tarballs and adjust types
entry points
With PR #4724, we introduced separate building of type declaration files which would be written to
<sdk>/build/types
. Furthermore, we adjusted thetypes
entry point in thepackage.json
s to this new directory. To avoid a breaking change, we kept type declarations indist
andesm
. However, a bug in this PR caused a small regression in most of our SDKs: The new types directory was not included in most of our SDK tarballs. Subsequently, we released a hot fix via #4745 which simply reset the entry points back to their previous path (dist/index.d.ts
) in all SDKs.This PR now properly fixes this bug:
build/types
directory. All.npmignore
s were adjusted to include this directory.types
entry point in allpackage.json
s are now adjusted to the new types directoryIn addition, this PR makes slight adjustments w.r.t. creating tarballs to the following SDK projects to improve consistency:
postbuild.sh
to copy.npmignore
to./build
postbuild.sh
to Node will happen in a separate PR coming soon (TM).npmignore
instead offiles
property inpackage.json
.npmignore
to be more similar to our other packages.npmignore
to be more similar to our other packages.npmignore
fileA note for reviewing: with the exception of Angular and Browser (both in 1st commit), all adjustments are one commit per SDK. If necessary, feel free to go through the PR commit by commit. The changes per SDK are pretty isolated though.
Also, since a similar change caused a regression last time, I double checked all entry points and tarballs. Regardless, a second pair of eyes would probably be good on that end.
ref: https://getsentry.atlassian.net/browse/WEB-730
ref: https://getsentry.atlassian.net/browse/WEB-745