- 
          
- 
                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 thetypesentry point in thepackage.jsons to this new directory. To avoid a breaking change, we kept type declarations indistandesm. 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/typesdirectory. All.npmignores were adjusted to include this directory.typesentry point in allpackage.jsons 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.shto copy.npmignoreto./buildpostbuild.shto Node will happen in a separate PR coming soon (TM).npmignoreinstead offilesproperty inpackage.json.npmignoreto be more similar to our other packages.npmignoreto be more similar to our other packages.npmignorefileA 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