-
-
Notifications
You must be signed in to change notification settings - Fork 662
fix: export Tag type for TS declaration emit
#1243
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: main
Are you sure you want to change the base?
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.
Copilot wasn't able to review any files in this pull request.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@ethanresnick I'm unable to reproduce this issue, can you help me reproduce it. |
|
Yeah, I'll try to put together a full reproduction later this week |
|
I also have this issue, so will try to create a reproduction too (have had forever, not just v5) |
|
I tried to put together a reproduction, and discovered that the issue may actually be a bug in TS; see microsoft/TypeScript#60913 (comment). However, that repro in that TS issue also shows how type-fest is effected: it uses a simplified version of type-fest's Meanwhile, here's a very simple repro with tagged types in particular, where there's an error goes away with this PR: https://github.com/ethanresnick/type-fest-bug-repro/blob/main/src/index.ts |
|
@sindresorhus If you read the linked TS issue (#60913), it seems like what's really going on here is that adding the |
|
As it stands on the TS side, type-fest may have to either export many more of its internal utility types, or inline their definitions into the public types that use them. |
I may be able to better explain later what's going on here, when I have a chance to debug it further, but the TL;DR is that, as of v5, when a variable whose type contains a
Taggedtype gets exported from a file, TS's declaration emit often produces a lot of errors like:It seems like the fact that the tag symbol now is imported from a separate package somehow influences TS' decisions about how to (try to) emit type declarations for an export. The result is that the emitted declarations look something like this:
I.e., TS has reduced
Tagged<....>to an intersection ofTag<..> & ... & Base, but, becauseTagisn't itself exported, the declaration emit hits blows up with the above error. Before, what seemed to happen more often is that the declared exported type would not be as reduced, and would just reference the publicTaggedexport.The only fix I've found is to manually import and re-export the
tagsymbol fromtagged-tag:With that, the emitter is able to see that type-fest's
Tagtype is just a small wrapper type around the same exported symbol, so it totally inlinesTag's definition. I.e., the code above instead is emitted as:This works, but unfortunately, this import and re-export has to be added transitively — i.e., it also has to be added to every file that re-exports something derived from
xyz. In my codebase, I had to add this import and re-export of thetagsymbol to 50+ files, and I'm sure that most developers would never intuit that this is the workaround when given the compiler's somewhat inscrutable error message.So, this PR fixes at least most of the symptoms of the issue by exporting
Tagso that emitted declarations likeimport("type-fest").Tag<"UUID", never>work.