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

packages provide wrong types under node16 module resolution #1031

Closed
not-my-profile opened this issue Jul 4, 2023 · 6 comments · Fixed by #1200
Closed

packages provide wrong types under node16 module resolution #1031

not-my-profile opened this issue Jul 4, 2023 · 6 comments · Fixed by #1200

Comments

@not-my-profile
Copy link

node16 from ESM: type errors

Using the packages from Deno with TypeScript leads to type errors, but these errors can also be observed with tsc and Node.JS when using the new node16 module resolution:

$ npm i postcss-nesting
$ echo '{"type": "module"}' > package.json
$ echo "import postcssNesting from 'postcss-nesting'; postcssNesting();" > example.ts
$ tsc example.ts --moduleResolution node16 --lib es2020
example.ts:1:47 - error TS2349: This expression is not callable.
  Type 'typeof import("/tmp/example/node_modules/postcss-nesting/dist/index")' has no call signatures.

1 import postcssNesting from 'postcss-nesting'; postcssNesting();
                                                ~~~~~~~~~~~~~~

node16 from CJS: runtime error

$ npm i postcss-nesting
$ echo '{"type": "commonjs"}' > package.json
$ echo "import postcssNesting from 'postcss-nesting'; postcssNesting();" > example.ts
$ tsc example.ts --moduleResolution node16 --lib es2020
$ node example.js
/tmp/example/example.js:4
(0, postcss_nesting_1.default)();
                              ^

TypeError: (0 , postcss_nesting_1.default) is not a function
    at Object.<anonymous> (/tmp/example/example.js:4:31)
    at Module._compile (node:internal/modules/cjs/loader:1254:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
    at Module.load (node:internal/modules/cjs/loader:1117:32)
    at Module._load (node:internal/modules/cjs/loader:958:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47

Node.js v18.14.2

What to do

As Are the types wrong? explains:

A golden rule of declaration files is that if they represent a module—that is, if they use import or export at the top level—they must represent exactly one JavaScript file. They especially cannot represent JavaScript files of two different module formats.

So the exports like the following are wrong since they provide one types for both ESM and CJS:

"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.mjs",
"require": "./dist/index.cjs",
"default": "./dist/index.mjs"
}
},

Unfortunately @rollup/plugin-typescript like apparently most other build tools does not supporting correctly emitting different type definitions for ESM and CJS.

(Note that the Are the types wrong? website currently wrongly doesn't report the node16 From CJS problem since it doesn't yet perform type checking).

@romainmenke
Copy link
Member

Hi @not-my-profile,
Thank you for reaching out about this.

Unfortunately this is impossible for us to solve.
This is an upstream problem.

As package authors we need tools to do this thing :

  • author code in TypeScript and ES Modules
  • transpile out to plain JS and ES Modules
  • transpile out to plain JS and commonjs
  • emit types for plain JS and ES Modules
  • emit types for plain JS and commonjs

No such tool exists.

You can report this issue either with TypeScript or with Rollup.


If however such a tool does exist, please let us know.
We are happy to change our setup to make this possible.

@not-my-profile
Copy link
Author

Thanks for the quick reply! I see. Yeah I think this currently hangs at microsoft/TypeScript#54593.

@CobyPear
Copy link

CobyPear commented Nov 8, 2023

Hi. I just ran into this issue as well.

No such tool exists.

I believe tsup as of the latest version (7.2.0) does a good job at making hybrid cjs and esm bundles with proper types.
Happy to look into contributing this change but I would need to find some time.

@romainmenke
Copy link
Member

Hi @CobyPear,
Thank you for this suggestion!

tsup does not look like a good solution.
It seems to be a collage of other tools that all solve bits and pieces.
I've spotted :

  • rollup
  • swc
  • esbuild
  • typescript
  • custom transforms

I fear it will be too hard to maintain such a dependency long term.
I prefer to wait for something more stable from a single provider.

@CobyPear
Copy link

CobyPear commented Nov 8, 2023

@romainmenke I think that's the world we live in right now in the TypeScript/Node ecosystem: pain for the end user because the package emits the wrong stuff, or pain for the maintainer to emit the right stuff.

I understand wanting a more robust solution to do this by default but I had thought in the past the TS team said they didn't want to deal with this issue and it was up to the ecosystem to make ESM, CJS, and TypeScript play nicely. I'll keep searching for the issue I'm referring to in the TypeScript repo and post it here if I find it. Maybe all that's needed for your libs is esModuleInterop set to true?

That article mentions the following which may be at play here:

allowSyntheticDefaultImports without esModuleInterop should be avoided.

This was referenced Dec 4, 2023
@romainmenke
Copy link
Member

romainmenke commented Dec 10, 2023

I think I currently have a good grasp on the complexity of this issue.
As I see it now :

  1. default exports/imports work differently in commonjs and es modules. They also work differently in typings for commonjs and es modules. Emitting types for anything that has default exports will be broken when dual publishing.
  2. rollup has a good hack for default exports when transpiling to commonjs. This hides any issues at runtime. TypeScript isn't aware that this hack is happening, so the types do not match the actual code after transpiling to commonjs.
  3. TypeScript can not express commonjs modules that have both a default export and a named export without applying much more complex and brittle hacks. There is no way to tell TypeScript it must do these hacks. You can only get there by manually writing .d.ts files. We have named exported types (i.e. pluginOptions) and default exported JavaScript (i.e. creator). This isn't compatible with commonjs typings.
  4. TypeScript looks in package.json for "type": "module" to decide if it should output commonjs or es modules code. There is no way to set this programmatically. So it is impossible to make TypeScript the driver without messing with your package.json during your build process.
  5. rollup only supports es modules and although it can output commonjs it doesn't have the right internals to process code end to end as commonjs. So it is impossible to let TypeScript output commonjs and then feed that to rollup for further processing.
  6. Modern TypeScript with a modern config expects file extensions in import statements. When dual publishing while transpiling from es modules TypeScript to commonjs and es modules JavaScript you simply do not know what the file extensions will be. So you can not have working imports of other .d.ts files.

Issues 1-5 are unsolvable by us.
We can switch to another build tool, but I've tried a few and you essentially get a different mix of issues.

But issues 1-5 go away when you stop dual publishing typings.
We can still do dual publishing of runtime code and simply omit types for commonjs.


Issue 6 can be solved by bundling types into a single .d.ts file.

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

Successfully merging a pull request may close this issue.

3 participants