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

Imports fail with TS "moduleResolution": "nodenext" #4991

Closed
simonszalai opened this issue Oct 6, 2023 · 8 comments · Fixed by #5668 or #5697
Closed

Imports fail with TS "moduleResolution": "nodenext" #4991

simonszalai opened this issue Oct 6, 2023 · 8 comments · Fixed by #5668 or #5697
Labels
help wanted Contributions from community are welcome

Comments

@simonszalai
Copy link

What package has an issue

@mantine/core

Describe the bug

When I set "moduleResolution": "nodenext" in tsconfig, TS throws an error for all Mantine imports. I am using the Epic Remix Stack from Kent C. Dodds, and if I go back to "moduleResolution": "node", Mantine imports work fine, but all kinds of other errors pop up all over the project. Now I'm just adding a @ts-ignore for each Mantine import, but that makes auto-import impossible, which is not great.

Is there a better workaround? What would it take to fix this?

What version of @mantine/* packages do you have in package.json? (Note that all @mantine/* packages must have the same version in order to work correctly)

7.1.2

If possible, please include a link to a codesandbox with the reproduced problem

No response

Do you know how to fix the issue

No

Are you willing to participate in fixing this issue and create a pull request with the fix

Yes

Possible fix

No response

@simonszalai
Copy link
Author

I just discovered that the types are correctly imported for @mantine/hooks, but not for @mantine/core and @mantine/dates.

I looked at the tsconfig for them, and there are differences:

for @mantine/core (does not work)

  "include": ["./src", "../../configuration/types"],
  "exclude": ["./src/mantine-core/lib/*.d.ts"],

for @mantine/hooks (does work)

  "include": ["./src", "./types", "../../configuration/types"],
  "exclude": ["./lib", "./esm", "./cjs"],

Could this be the problem?

It is actually a very annoying bug, as I practically can't use TypeScript with anything imported from @mantine/core, and I need to put //@ts-ignore to too many places in my codebase. I hope it's an easy fix and can be patched soon!

@rtivital rtivital added the help wanted Contributions from community are welcome label Oct 12, 2023
@waweber
Copy link
Contributor

waweber commented Oct 29, 2023

I have also run into this. Attached is a minimal project that shows the issue.
Archive.zip. You will see that tsc reports Module '"@mantine/core"' has no exported member 'TextInput'..

Here's what's going on:

  • Our project is "type": "module", so our .ts files are ES modules.
  • Our TSConfig is using the modern NodeNext resolution.
  • We want to resolve TextInput from @mantine/core.
  • We look in node_modules/@mantine/core and find package.json.
  • No "type" is set, so whether a file is considered an ES module or CommonJS module will be based on the extension.
  • Since we're using import and we're resolving types, we'll look at the package's "exports" and pick the entry "./lib/index.d.mts"
  • We look inside this file, and since it is a .d.mts file, we treat it as an ES module.
  • This file re-exports TextInput like this: export * from './components';
  • Because this file is treated as an ES module, our NodeNext resolution rules apply. Mainly, it does not support implicit index.js, and all imports that refer to files must include the extension.
  • Resolution fails here because a module named ./components cannot be found due to those two rules.

Phew! Now what can we do to fix this? There's a few things that can work:

  • Change the "exports" field for the types to refer to the index.d.ts file instead of a separate index.d.mts file. These files are both identical, but the .d.mts extension is what causes the new rules to apply. Keeping it as .d.ts allows the relative ./components import to work like before with the implicit index.js. I'm not sure what effects this has on those with CommonJS projects or non-NodeNext settings though.
  • Output an entirely separate set of .d.mts files for the ESM version of the files. This is probably the most correct option for now, but it would require the build process to add explicit index.d.mts and .d.mts extensions to the imports.

It looks like there's already something in the build process for rewriting imports, so either way this is probably a simple fix.

@waweber
Copy link
Contributor

waweber commented Oct 29, 2023

It looks like in #4792 there were some issues that showed up due to other packages assuming the wrong module format, so the .d.mts extension will have to stay.

I assume just the main index file being ".d.mts" was enough? If so, only the imports in that file need to have their imports adjusted, like export * from './components/index.js' (or .d.ts?)

@waweber
Copy link
Contributor

waweber commented Oct 30, 2023

@simonszalai in the meantime, try this workaround in your tsconfig.json:

"compilerOptions": {
   "paths": {
       "@mantine/core": ["./node_modules/@mantine/core/lib/index.d.ts"]
   }
}

@simonszalai
Copy link
Author

@waweber amazing, that worked, thank you!

@sid374

This comment was marked as off-topic.

@waweber
Copy link
Contributor

waweber commented Jan 2, 2024

@sid374 I would guess its something like the datatable library resolving and importing a different copy of the mantine libs due to the workaround. I haven't had time, but in a few weeks I think I will poke around with the build scripts to come up with an actual patch if someone else doesn't get to it

@waweber
Copy link
Contributor

waweber commented Feb 9, 2024

@simonszalai the fix for this is included in version 7.5.2, so try it out and see if it works now without the workaround

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Contributions from community are welcome
Projects
None yet
4 participants