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

Split TS declaration files are not compatible with nodenext module resolution #1019

Closed
ssangervasi opened this issue Jan 17, 2023 · 3 comments

Comments

@ssangervasi
Copy link

TLDR: Microbundle should output a single .d.ts per output module file, or nodenext is too unstable to target at all.

Background

This issue is a funky mixture of a few other issues:

  1. The ongoing changes to TS's module resolution, where nodenext (aka node16) uses modules: A Proposal For Module Resolution microsoft/TypeScript#50152
  2. Microbundle's basic documentation of nodenext compat: Guidance around exports.types config when authoring in TS #990
  3. And the fact microbundle generates multiple .d.ts files for a single .*js output: [Typescript] Typings files not concatenated? #239

Reproducing

I forked the code from #239 and updated it to highlight the problem with nodenext: https://github.com/ssangervasi/microbundle-typings.
In this example, you can see:

  1. ✔️ The "library" package is built with microbundle
  2. ✔️ The library package has exports.types pointing to the module's types, dist/index.d.ts, as suggested by microbundle's readme.
  3. 😐 That declaration file has a relative dependency on another file produced by microbundle.
  4. ✔️ The dependent package is built with tsc 4.9.4 with nodenext. (This is the thing I want the users of my package to be able to do.)
  5. ✔️ The dependent package can resolve the library package's root declaration.
  6. 😞 TypeScript can't resolve the relative dependency produced by microbundle.

Proposed solution

My instinct is that generating multiple .d.ts files for a single output module is going to cause problems in general. In this case, if the build process could merge them (the same way it merges the JS into a single file), the TS resolution problem would go away. Basically reopening #239.

On the other hand, TS's moduleResolution is madenning*, and it's possible this relative lookup will go away in later versions. Microbundle's readme could be updated to note this incompatibility for now. As far as I can tell, there isn't a way to get these split .d.ts files to work, except maybe package.json redirects which I'm not willing to try or publish.

* On par with how complex JS's dependency system is.

@rschristian
Copy link
Collaborator

rschristian commented Jan 17, 2023

There's an easy fix:

// src/index.ts
-export * from "./foo";
+export * from "./foo.js";

Longer answer: This really isn't a Microbundle issue, but a TS issue. You'll run into this same behavior using tsc directly. See microsoft/TypeScript#16577, microsoft/TypeScript#49083

The TS team have drawn a line in the sand and refuse to rewrite module specifiers as there's no TS features there; instead, you're supposed to use .js to refer to .ts and .tsx files. There have been dozens of issues opened up on the TS repo for this, but it's the way they've decided to go. We're (more or less) using tsc here, so our behavior matches the official compiler.

While merging declaration files could be nice, it's not without it's own complexities and it would be a move away from tsc which increases the maintenance burden. Outputting multiple .d.ts files for a single output is default tsc behavior, it's not a choice we made or anything.

@ssangervasi
Copy link
Author

Holy moly, I read about that but still couldn't understand the fix applied to this situation.

TypeScript will impose Node’s much stricter ESM resolution algorithm on those files, disabling index-file resolution and extensionless lookups—in fact, the extension the user has to write is .js, which will be nonsensical for the context, where the runtime module resolver (the bundler) only ever sees .ts files.

Thanks for explaining!

@rschristian
Copy link
Collaborator

No problem, it's certainly a weird one. Using foo.js to refer to foo.d.ts file is quite the odd design choice to most users I think.

trygveaa added a commit to trygveaa/KTX-Parse that referenced this issue Feb 21, 2023
When importing this library in a project using moduleResolution
nodenext, the types would give errors because the import paths were
missing the file extension which is required for moduleResolution
nodenext. Therefore we have to add .js to all imports of local files.

See developit/microbundle#1019 (comment)
and the issues that comment links to for some more detailed explanation
of this.

I also changed moduleResolution to nodenext in the tsconfig because when
it's set to node, typescript won't give errors for missing file
extensions which means they are easy to forget. With it set to nodenext,
you will get an error if an import is missing the file extension. As far
as I can see, changing this doesn't change the build output.
trygveaa added a commit to trygveaa/property-graph that referenced this issue Feb 21, 2023
When importing this library in a project using moduleResolution
nodenext, the types would give errors because the import paths were
missing the file extension which is required for moduleResolution
nodenext. Therefore we have to add .js to all imports of local files.

See developit/microbundle#1019 (comment)
and the issues that comment links to for some more detailed explanation
of this.

I also changed moduleResolution to nodenext in the tsconfig because when
it's set to node, typescript won't give errors for missing file
extensions which means they are easy to forget. With it set to nodenext,
you will get an error if an import is missing the file extension. As far
as I can see, changing this doesn't change the build output.
trygveaa added a commit to trygveaa/glTF-Transform that referenced this issue Feb 21, 2023
When importing this library in a project using moduleResolution
nodenext, the types would give errors because the import paths were
missing the file extension which is required for moduleResolution
nodenext. Therefore we have to add .js to all imports of local files.

See developit/microbundle#1019 (comment)
and the issues that comment links to for some more detailed explanation
of this.

I also changed moduleResolution to nodenext in the tsconfig because when
it's set to node, typescript won't give errors for missing file
extensions which means they are easy to forget. With it set to nodenext,
you will get an error if an import is missing the file extension. As far
as I can see, changing this doesn't change the build output.

Depends on donmccurdy/property-graph#70 and
donmccurdy/KTX-Parse#55

Fixes donmccurdy#824
trygveaa added a commit to trygveaa/glTF-Transform that referenced this issue Feb 21, 2023
When importing this library in a project using moduleResolution
nodenext, the types would give errors because the import paths were
missing the file extension which is required for moduleResolution
nodenext. Therefore we have to add .js to all imports of local files.

See developit/microbundle#1019 (comment)
and the issues that comment links to for some more detailed explanation
of this.

I also changed moduleResolution to nodenext in the tsconfig because when
it's set to node, typescript won't give errors for missing file
extensions which means they are easy to forget. With it set to nodenext,
you will get an error if an import is missing the file extension. As far
as I can see, changing this doesn't change the build output.

Depends on donmccurdy/property-graph#70 and
donmccurdy/KTX-Parse#55

Fixes donmccurdy#824
trygveaa added a commit to trygveaa/glTF-Transform that referenced this issue Feb 21, 2023
When importing this library in a project using moduleResolution
nodenext, the types would give errors because the import paths were
missing the file extension which is required for moduleResolution
nodenext. Therefore we have to add .js to all imports of local files.

See developit/microbundle#1019 (comment)
and the issues that comment links to for some more detailed explanation
of this.

I also changed moduleResolution to nodenext in the tsconfig because when
it's set to node, typescript won't give errors for missing file
extensions which means they are easy to forget. With it set to nodenext,
you will get an error if an import is missing the file extension. As far
as I can see, changing this doesn't change the build output.

Depends on donmccurdy/property-graph#70 and
donmccurdy/KTX-Parse#55

Fixes donmccurdy#824
donmccurdy pushed a commit to donmccurdy/property-graph that referenced this issue Feb 22, 2023
When importing this library in a project using moduleResolution
nodenext, the types would give errors because the import paths were
missing the file extension which is required for moduleResolution
nodenext. Therefore we have to add .js to all imports of local files.

See developit/microbundle#1019 (comment)
and the issues that comment links to for some more detailed explanation
of this.

I also changed moduleResolution to nodenext in the tsconfig because when
it's set to node, typescript won't give errors for missing file
extensions which means they are easy to forget. With it set to nodenext,
you will get an error if an import is missing the file extension. As far
as I can see, changing this doesn't change the build output.
donmccurdy pushed a commit to donmccurdy/KTX-Parse that referenced this issue Feb 22, 2023
When importing this library in a project using moduleResolution
nodenext, the types would give errors because the import paths were
missing the file extension which is required for moduleResolution
nodenext. Therefore we have to add .js to all imports of local files.

See developit/microbundle#1019 (comment)
and the issues that comment links to for some more detailed explanation
of this.

I also changed moduleResolution to nodenext in the tsconfig because when
it's set to node, typescript won't give errors for missing file
extensions which means they are easy to forget. With it set to nodenext,
you will get an error if an import is missing the file extension. As far
as I can see, changing this doesn't change the build output.
trygveaa added a commit to trygveaa/glTF-Transform that referenced this issue Feb 22, 2023
When importing this library in a project using moduleResolution
nodenext, the types would give errors because the import paths were
missing the file extension which is required for moduleResolution
nodenext. Therefore we have to add .js to all imports of local files.

See developit/microbundle#1019 (comment)
and the issues that comment links to for some more detailed explanation
of this.

I also changed moduleResolution to nodenext in the tsconfig because when
it's set to node, typescript won't give errors for missing file
extensions which means they are easy to forget. With it set to nodenext,
you will get an error if an import is missing the file extension. As far
as I can see, changing this doesn't change the build output.

Depends on donmccurdy/property-graph#70 and
donmccurdy/KTX-Parse#55

Fixes donmccurdy#824
donmccurdy pushed a commit to donmccurdy/glTF-Transform that referenced this issue Feb 22, 2023
When importing this library in a project using moduleResolution
nodenext, the types would give errors because the import paths were
missing the file extension which is required for moduleResolution
nodenext. Therefore we have to add .js to all imports of local files.

See developit/microbundle#1019 (comment)
and the issues that comment links to for some more detailed explanation
of this.

I also changed moduleResolution to nodenext in the tsconfig because when
it's set to node, typescript won't give errors for missing file
extensions which means they are easy to forget. With it set to nodenext,
you will get an error if an import is missing the file extension. As far
as I can see, changing this doesn't change the build output.

Depends on donmccurdy/property-graph#70 and
donmccurdy/KTX-Parse#55

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

No branches or pull requests

2 participants