Skip to content
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

fix(types): update type deps #1776

Merged
merged 3 commits into from
Aug 17, 2022

Conversation

thw0rted
Copy link
Contributor

Remove duplicate long types

Long has shipped with its own TS types for a while now, and the package from DefinitelyTyped conflicts with the newest version. TS consumers wind up with errors when trying to work with Long values.

Remove duplicate `long` types
@alexander-fenster alexander-fenster changed the title Update type deps fix(types): update type deps Jul 20, 2022
@alexander-fenster
Copy link
Contributor

@thw0rted Would you mind fixing the failing tests in your branch? Thank you!

@thw0rted
Copy link
Contributor Author

I think it's fixed now. I updated the test-runner script to do the whole directory at once, rather than 3 separate invocations of tsc. That should be enough to close out this PR.

I did also try to take the "minimal" definition of a Long interface out of util/common.js but vanilla JSDoc still doesn't have a way to import a type definition (jsdoc/jsdoc#1537, jsdoc/jsdoc#1645) so I think we're stuck with it for now.

Just as an aside, this is like the 4th issue in the last week where trying to stick with vanilla JSDoc, instead of using TypeScript's JSDoc parser, has made my life difficult. JSDoc is a dead standard, they haven't published a meaningful update since 2017, and it's really holding the whole ecosystem back. Specifically, ESM support has come a long way in 5 years, and JSDoc still assumes a CJS / global-pollution model for types... and we have no reason to expect them to rectify that situation. Not sure what PBJS can do about that, I guess, but it's something to think about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants