-
Notifications
You must be signed in to change notification settings - Fork 9
fix: ensure we include the types file when publishing #38
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe package.json file was modified to add a "types" field pointing to "index.d.ts", specifying the location of TypeScript type declarations for the package. Additionally, the "files" array was updated to include "index.d.ts" along with the existing "src" directory, ensuring that the type declaration file is included when the package is published. No other parts of the package.json or source code were changed. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
9-9: Correct addition of thetypesfield – consider adding legacy alias for wider tooling supportAdding
"types": "index.d.ts"will let TS-aware consumers pick up your declarations—nice. For maximum backwards-compatibility with older tooling that still reads"typings", you could add the alias:"main": "src/index.js", "types": "index.d.ts", + "typings": "index.d.ts",It is harmless duplication and keeps pre-2.0 TypeScript tooling happy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json(1 hunks)
🔇 Additional comments (1)
package.json (1)
10-13:filesarray update looks good – double-check.npmignoredoesn’t undo itIncluding
index.d.tsin thefileswhitelist ensures the tarball ships with the declarations. Just confirm there isn’t an.npmignorethat excludes*.d.ts, otherwise it will override thefilesentry. A quicknpm pack --dry-runwill verify the output.
📚 Context/Description Behind The Change
Thanks for this project! I enjoyed it while at Mixmax and afterwards, too.
I appreciate y'all adding a types file, this PR ensures that the types file
index.d.tsis included in the final built tarball and also referenced in a manner that can be consumed by other TypeScript modules. As it currently is, TypeScript code will complain that there are no useable types and refer one to try to install from the@types/*wild world, but there is no type there either.🚨 Potential Risks & What To Monitor After Deployment
There is no major risk here, worst case the final tarball is corrupted somehow - but that should not be feasible with this change.
🧑🔬 How Has This Been Tested?
There are no major tests here as there isn't a build or bundle script that's necessary, but I made sure that I ran local linting and testing scripts.
🚚 Release Plan
Y'all should just need to publish this however y'all are - I don't see a CI release pipeline setup, or that would likely be all that's needed.