Issues importing with Node16/NodeNext moduleResolution about valibot HOT 35 CLOSED

I think I could solve the problem now quite easily by using tsup as the new build step. Thank you all for the info und help!

from valibot.

@intrnl something like this?

from valibot.

It seems like you all got this figured out, but I actually just wrote a bit of additional documentation for TS about this yesterday that might explain what’s going on:

Considerations for bundling libraries

If you’re using a bundler to emit your library, then all your (non-externalized) imports will be processed by the bundler with known behavior, not by your users’ unknowable environments. In this case, you can use "module": "esnext" and "moduleResolution": "bundler", but only with a significant caveat: you must ensure that your declaration files get bundled as well. Recall the first rule of declaration files: every declaration file represents exactly one JavaScript file. If you use "moduleResolution": "bundler" and use a bundler to emit an ESM bundle while using tsc to emit many individual declaration files, your declaration files may cause errors when consumed under "module": "nodenext". For example, an input file like:

import { Component } from "./extensionless-relative-import";

will have its import erased by the JS bundler, but produce a declaration file with an identical import statement. That import statement, however, will contain an invalid module specifier in Node.js, since it’s missing a file extension. For Node.js users, TypeScript will error on the declaration file and infect types referencing Component with any, assuming the dependency will crash at runtime.

from valibot.

Related issue: #2

The proposed solution on the related issue shouldn't be taken for granted as switching between moduleResolution values affects a lot more than just making valibot work.

from valibot.

Thanks for the tip. Do you know what I need to do to fix this? Here is my Vite build config: https://github.com/fabian-hiller/valibot/blob/main/library/vite.config.ts

from valibot.

move the types to be the first one in the package.json exports: https://publint.dev/rules#exports_types_should_be_first

from valibot.

Have updated it. Will release a new version later.

from valibot.

Moving the types into the first condition only fixes the fallback condition warning, Node16 and NodeNext still has an issue with the types not resolving.

from my previous experience, this involves authoring the library in Node16 to begin with, I'm not too sure about other solutions

@andrewbranch heya, I hope pinging you here doesn't bother you much, is there any solutions for Node16/NodeNext import problems that doesn't require rewriting every imports to use .js?

from valibot.

Making the project authored using Node16 and rewriting every single source files (346 files total) to have .js suffix when importing other modules fixes the issue with Node16 at least from the ESM side, but TypeScript thinks that it's only being authored in ESM and not dual-package.

from what I can remember, the solution that another project took is to generate the TypeScript declarations twice, one for ESM, and one for CJS

fixing the Node16/NodeNext module resolution here is important because not only that's how Node.js 16+ resolves modules, a lot of Vite projects had been bootstrapped using that moduleResolution value prior to the addition of Bundler

from valibot.

Does the problem only affect the TypeScript types of the library? If so, what does this have to do with Node? I have not had this problem yet.

In the build step, I put all the source code into a single file. Therefore there are no imports and .js should not be the problem. In the dist directory you will find the .cjs and .mjs file: https://www.npmjs.com/package/valibot?activeTab=code

from valibot.

the reason why I had mentioned Node.js alongside Vite is because this issue affects users that would want to use the library on their Node.js apps, whether it'd be CLI, servers etc, so unless they're using a bundler, they would be using Node16/NodeNext for module resolution when authoring a project with "type": "module" or Node.js' ESM

from valibot.

@fabian-hiller I'll try to fix it, what version of pnpm are you using? it is missing from the package.json file

from valibot.

Can you check if the problem is fixed with v0.2.1?

from valibot.

assuming the "internal resolution error" is indeed because of the missing .js suffixes on imports, and the following solution is the only way to properly solve this issue for Node16 users:

Internal resolution error
This problem can occur with tsc alone if the library author compiles with --moduleResolution node (also known as node10) or bundler, especially in combination with a package.json that has "type": "module". Library authors should always compile/check with --module node16 --moduleResolution node16 or nodenext (note that module implies the corresponding moduleResolution value, but not vice versa), because this is the only TypeScript mode that recognizes Node’s strict module resolution algorithm for ESM.

then the following commit fixes this: intrnl@95db56f

this fixes Node16 from the ESM side, fixing the CJS side would mean generating another set of declarations for CJS.

let me know if this is the solution to go forward with, and I'll make the pull request

from valibot.

@userquin I use pnpm v8.6.3

from valibot.

Before we add .js everywhere, I would like to understand how other libraries have solved this. @intrnl do you know how e.g. Zod fixed it?

from valibot.

it seems like Zod is masquerading type declarations meant for CJS as ESM, I can try doing that as an alternative solution but it seems like there are some pitfalls that you might need to be aware of

from valibot.

it seems you've a few more submodules and you're exporting the types from types submodule instead types module

EDIT: you should add the submodules to the exports, I'm using unbuild to build the library

from valibot.

@userquin what do you mean by submodule? Can you make an example and show me what do you think that we should change?

from valibot.

@intrnl if I understand it correctly, the problem only affects the TypeScript types and not the JavaScript code, right? Currently I export the types under /dist/types. Can't we just export the types for node16 separately and reference them in the package.json? Then no change with .js to the source code would be necessary.

from valibot.

@fabian-hiller Yes, the actual library code is fine, the issue here is only with the TypeScript types.

I've followed roughly with what Zod is doing and it seems to work well here: intrnl@e48698a
If the consequences described here is worth not having to reauthor the entire project as Node16 then it might be a good solution

from valibot.

@intrnl does it also work if you do not change the tsconfig.json and only make changes to the package.json and set main to ./dist/index.cjs?

from valibot.

only changing this one line?

no it does not fix the issue, TypeScript doesn't take account of main when types is involved

from valibot.

@intrnl yes, but to ./dist/index.cjs:

  "main": "./dist/index.cjs",
  "module": "./dist/index.mjs",
  "types": "./dist/types/index.d.ts",

In my tests, the types were exactly the same. Only the JavaScript files changed from import to require.

from valibot.

yeah, I switched to a fresh branch and only changed the main field to ./dist/index.cjs, the result is as above

from valibot.

In the code above you wrote .mjs instead of .cjs. If you change that, does it work?

from valibot.

No, same result. I'm somewhat failing to see why this would fix it since TypeScript is only concerned about the types here, and since the types field is provided, it will only resolve that.

from valibot.

I mean this (@intrnl can you check with attached tgz via file protocol (rename it to tgz extension) ?):

valibot-0.2.0.zip

from valibot.

I will try to investigate the problem tonight and then make a decision.

from valibot.

we can remove all the submodules to just keep the index, I'm using unbuild, tsc used only to copy the sources: I guess the problem is the index modules not using types generated by tsc.

There is no index.d.ts generate by Vite, only the cjs and mjs modules, then tsc will generate the types inside the types folder.

from valibot.

I can send a PR if you want to take a look, I'll add instructions to move to multiple submodules or to just one.

from valibot.

#12

This means that for anyone creating a TypeScript CJS project on Node.js 16+, they'd have to use await import to properly get the types of valibot even if there's a CJS build available

from valibot.

we can add require and import custom entries, if you check the cjs files you don't need to use import nor await, we need to add the cts file(s) and add the types for require

from valibot.

I'll update the PR to add the d.mts and d.cts files and update the export entries, this way we can use vitebot with CJS or ESM, it will take a while.

from valibot.

@intrnl PR updated if you want to build the tgz and test in your local via file or link protocol

from valibot.