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

Possible issue with missing type declaration files for ESM exports #1085

Open
aholachek opened this issue Jul 6, 2024 · 15 comments
Open

Possible issue with missing type declaration files for ESM exports #1085

aholachek opened this issue Jul 6, 2024 · 15 comments

Comments

@aholachek
Copy link

Hi, this is possibly user error, but a recent routine release of my lib react-flip-toolkit broke type declarations-- a user filed an issue here and when I view the analysis on this page I see that types are not found.

After digging around for a bit, it seems possible based on my reading of this explanation that we need to package an index.d.mts alongside the index.d.ts automatically generated by microbundle.

@rschristian
Copy link
Collaborator

rschristian commented Jul 6, 2024

a user filed an issue here and when I view the analysis on this page I see that types are not found.

This honestly seems like a TS bug as you don't seem to use "exports" at all. I don't know why TS wouldn't resolve then to the top-level "types" entry.

we need to package an index.d.mts alongside the index.d.ts automatically generated by microbundle.

We don't support this at the moment, see #1030

@aholachek
Copy link
Author

Sorry for the lack of clarity in my question--I ended up removing exports in a commit this morning because that was the only way I could solve the problem.

Until today my exports field in my package.json looked like this.

@aholachek
Copy link
Author

Also be honest I am very flummoxed about why this issue just popped up just now, since I haven't changed anything major about my microbundle config since I added the modern export a bit more than a year ago!

@rschristian
Copy link
Collaborator

rschristian commented Jul 6, 2024

Ah, that makes sense. Thanks.

Do you want to supports CJS through "exports"? If not, we can handle that:

"exports": {
  "types": "./lib/index.d.mts",
  "default": "./lib/index.modern.mjs"
}

Keep in mind that "types" needs to come first.

why this issue just popped up just now

This happens when someone sets "moduleResolution": "node16" in their tsconfig.json, it changes the resolution process.

@aholachek
Copy link
Author

Oh wow gotcha, thanks. And thanks so much for the quick response here.

Do you want to supports CJS through "exports"?

I am not sure, I think I want to focus on just maintaining current functionality, which seems like does not support CJS, right?
I think I need to spend more time learning about the different module options.

@aholachek
Copy link
Author

I will try the solution you outlined and report back! I think I must have had them out of order before.

@rschristian
Copy link
Collaborator

I am not sure, I think I want to focus on just maintaining current functionality, which seems like does not support CJS, right?

Yep, this is ESM-only (through "exports", anyhow. You do still have a CJS entry via "main"):

"exports": "./lib/index.modern.mjs",

And this is then the correct TS representation:

"exports": {
  "types": "./lib/index.d.mts",
  "default": "./lib/index.modern.mjs"
}

As TS needs to know where to find the types for this.

Please note I made a typo in the comment above, I didn't see .mjs. You'll need to do a post-build copy to .d.mts as TS made a rather frustrating change.

@aholachek
Copy link
Author

aholachek commented Jul 6, 2024

Following your instructions seemed to create another error in the are the types wrong website, you can see the new report here.

And here's the newly published package.json with your instructions integrated.

Thanks for your help so far, this is not super urgent because I can just stick with the removed modern build...

@aholachek
Copy link
Author

Also I should say just for fun I tried "moduleResolution": "bundler", in my tsconfig.json, but that did not seem to help either.

@rschristian
Copy link
Collaborator

Oh shoot, try switching "default" over to "import" -- I believe "default" is meant to match the module type.

@aholachek
Copy link
Author

I unfortunately still see the same errors as before on arethetypeswrong

@rschristian
Copy link
Collaborator

rschristian commented Jul 6, 2024

(Last guess, I promise, feel free to ignore until I can check myself): It might be the (somewhat wonky) need to use file extensions even in the .d.ts. Here's what exists now:

import * as utilities from './utilities';
import * as constants from './constants';
export { default as Flipper } from './Flipper';
export { default as getFlippedElementPositionsBeforeUpdate } from './flip/getFlippedElementPositions/getFlippedElementPositionsBeforeUpdate';
export * from './flip';
export { utilities, constants };
export { default as spring } from './Spring';

I think TS's ESM support requires that turn into this:

import * as utilities from './utilities.js';
import * as constants from './constants.js';
export { default as Flipper } from './Flipper.js';
export { default as getFlippedElementPositionsBeforeUpdate } from './flip/getFlippedElementPositions/getFlippedElementPositionsBeforeUpdate.js';
export * from './flip.js';
export { utilities, constants };
export { default as spring } from './Spring.js';

Even if some of those are .d.ts files, you'll need to use .js in the import specifier. It's a bit weird.

Sorry I haven't been able to get you a fix, I'll be able to clone & test locally in an hour or so.

@rschristian
Copy link
Collaborator

rschristian commented Jul 6, 2024

Alright, it's indeed that.

ESM requires full file paths (i.e., utilities/index.js instead of utilities) which TS mirrors with "moduleResolution": "node16", and TS, in their desire to be purely additive, has decided they will not correct nor alter paths in any way. Your types now have to act like "real ESM", if you will.

So what you need to do is "correct" your entry point to look like this:

// src/index.ts
import * as utilities from './utilities/index.js'
import * as constants from './constants.js'
export { default as Flipper } from './Flipper.js'
export { default as getFlippedElementPositionsBeforeUpdate } from './flip/getFlippedElementPositions/getFlippedElementPositionsBeforeUpdate/index.js'
export * from './flip/index.js'
export { utilities, constants }
export { default as spring } from './Spring/index.js'

And do the same across your entire project. Yes, this means you're referring to (say) ./utilities/index.ts with ./utilities/index.js in your source (and your built types will then also use .js to then refer to .d.ts files), which is a bit silly, but that's the way the TS team wants people to do things. See mega thread of confusion and questioning.

We do use TS itself to output types, which unfortunately means we can't correct these paths for you.

Sorry, I know this is a pain (to put it lightly).

@aholachek
Copy link
Author

aholachek commented Jul 7, 2024

Thanks for digging into this and for linking the thread. I will look at your pr--it feels like such a hack that we have to add file extensions like that, I never would have figured this out in a million years. I really appreciate your time looking into this today.

@rschristian
Copy link
Collaborator

it feels like such a hack that we have to add file extensions like that

I 100% agree.

There's a couple things in newer TS versions that help somewhat, namely, allowing you to write .ts & .tsx in your import specifiers, which was previously disallowed, though TS itself still won't fix them on build (need other tooling to do that). I'd love to see a "please just handle this for me option" but doesn't seem like one's landed yet. There are some other build tools that attempt it, I can't speak for any of them, but it's very possible something other than Microbundle might address your circumstances better.

Sorry for the long list of things to check & that the solution, as far as I understand it, is a bit miserable.

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

No branches or pull requests

2 participants