-
Notifications
You must be signed in to change notification settings - Fork 326
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
Consider JSDoc @import
for non-exported types
#5239
base: main
Are you sure you want to change the base?
Conversation
Oh, that's nice, thanks for pointing that out and updating the existing |
@romaricpascal No problem 😊 Nah only in "draft" because I've only rolled out I'd recommend widely using Just pushed an extra commit to demonstrate |
Thanks for marking as ready for review. I'll approve the workflows and have a look Thursday or Friday, hopefully it'll play nicely with the TypeDoc config update (whose aim is to have the types we don't export in the doc so you'd know what shape an Happy to take over updating the rest of the
Just to confirm, is it this part of the commit that statement refers to, where you were able to drop the |
Yeah go for it ✅
That's the one Previously all generics needed to be re-declared via |
4f599cd
to
0034633
Compare
0034633
to
7a2f945
Compare
Cheers! I've rebased on I've also added a round of updates for the Looking into this further, I'm thinking one of the effect of using For earlier versions,
This shouldn't break existing builds, just loosen the type checking a little (again, if I understand correctly). Will have a check with the team, but it doesn't feel like a blocker to start using |
This comment was marked as resolved.
This comment was marked as resolved.
1 similar comment
This comment was marked as resolved.
This comment was marked as resolved.
Think you're all set IDEs and editors will discover Thanks @romaricpascal |
7a2f945
to
e535334
Compare
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've rebased the PR on main and it looks good to me. It'll need a second approval before we can merge it though.
The one in `tasks/index.mjs` is left as it renames the import and there does not seem to be a syntax for that with the `@import` tag
Without it, internal types like `AccordionConfig` will be unresolved in the built files as the `@import` comments are dropped
e535334
to
bcdae6f
Compare
Hello 👋
Thought I'd share a little improvement added in TypeScript 5.5: JSDoc
@import
tagShould prevent
@typedef
re-exporting private/internal definitions in #5157 by using@import
instead