-
Notifications
You must be signed in to change notification settings - Fork 110
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
@mcap/support: remove .d.ts file in favor of module augmentation in source file #1318
@mcap/support: remove .d.ts file in favor of module augmentation in source file #1318
Conversation
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. Thanks!
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.
I would not bump the package version in the same PR - a habit I try to avoid should we decide on some other change being part of a new release
Even if I intend to release immediately after merging the PR? I find this slows down the ability to release since you have to wait for review again 🤷 |
Changelog
@mcap/support: Fixed a TS error from an import that did not work in the published package
Docs
None
Description
Supersedes / closes #1300
See also protobufjs/protobuf.js#1499
This area of TypeScript arcana is confusing, but I don't think there is anything about module augmentations that requires them to be in a
.d.ts
file.The way it's currently done was generally working, except as pointed out in #1300 we didn't include the
typings
dir in the published package.I believe we didn't run into this problem in our own code because we usually turn on
"skipLibCheck": true
.