plugins icon indicating copy to clipboard operation
plugins copied to clipboard

@rollup/plugin-node-resolve typings don't work with TypeScript 4.7's moduleResolution: Node16

Open michael42 opened this issue 3 years ago • 4 comments

  • Rollup Plugin Name: @rollup/plugin-node-resolve
  • Rollup Plugin Version: 13.3.0
  • Rollup Version: 2.74.1 (not relevant)
  • Operating System (or Browser): Linux (not relevant)
  • Node Version: 16.15.0 (not relevant)
  • Link to reproduction: https://github.com/michael42/rollup-plugin-node-resolve-typings

Expected Behavior

The typings supplied by @rollup/plugin-node-resolve should still work with TypeScript 4.7 when ECMAScript Module support for Node.js is enabled (i.e. "moduleResolution": "Node16", or NodeNext is configured).

Actual Behavior

TypeScript can't find the typings anymore, because is uses the contents of the exports-field to find them and the top-level types-filed is no longer used. This is analogous to how Node.js no longer uses the main or module fields when exports is present and is explained somewhat in the announcement blog post.

$ tsc # (npm run build in the reproduction repo)
rollup.config.js:1:25 - error TS7016: Could not find a declaration file for module '@rollup/plugin-node-resolve'. '/home/michael/Sync/Projects/rollup-plugin-node-resolve-typings/node_modules/@rollup/plugin-node-resolve/dist/es/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/rollup__plugin-node-resolve` if it exists or add a new declaration (.d.ts) file containing `declare module '@rollup/plugin-node-resolve';`

1 import nodeResolve from "@rollup/plugin-node-resolve"
                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 1 error in rollup.config.js:1

Additional Information

I experimented a bit with how to fix this.

Simply adding the types-field to exports, like this:

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

only fixes importing the named export (and is discouraged). Importing the default export (import nodeResolve from "@rollup/plugin-node-resolve") at first seems to work, but it doesn't result in a callable function. I think I understand now why that happens:

  • the package.json does not specify "type": "module", so .{js,ts}-files are specified as CommonJS
  • the index.d.ts file is not a .d.mts file and therefore assumed to be CommonJS
  • CommonJS basically exports a single unnamed export (that is made available as default export when getting imported)
  • so TypeScript thinks that the ES-Build of the plugin actually does module.exports = { default: /* ... */ } and therefore forbids directly calling the default export (it would allow calling nodeResolve.default, but that doesn't exist at run-time)

From what I understand, a complete fix would need to tell TypeScript, for which module system the typings are:

  • either by using the d.mts/d.cts-filenames
  • or by using a separate package.json with a different type-field (which already exists as dist/es/package.json)

Additionally, there's the question of either letting TypeScript find the typings automatically based on the file name or explicitly specifying the location of the typing files, which would look like this:

  "exports": {
    ".": {
      "import": {
        "types": "...",
        "default": "..."
      },
      "default": {
        "types": "...",
        "default": "..."
      }
    }
  }

But I think that is very verbose and error-prone, so I'd rather suggest keeping the exports-field as-is and switching to this file structure:

 @rollup/plugin-node-resolve
 ┣ package.json
 ┗ dist
   ┣ cjs
   ┃ ┣ index.js
   ┃ ┗ index.d.ts (referenced by the types-field)
   ┗ es
     ┣ package.json (as-is, simply {"type": "module"})
     ┣ index.js
     ┗ index.d.ts (new, identical to dist/cjs/index.d.ts)

The existing package.json can even be removed later on, by renaming the files like this:

 @rollup/plugin-node-resolve
 ┣ package.json
 ┗ dist
   ┣ cjs
   ┃ ┣ index.cjs
   ┃ ┗ index.d.cts
   ┗ es
     ┣ index.mjs
     ┗ index.d.mts

What do you think?

michael42 avatar May 22 '22 12:05 michael42

Looking at the source (rather than the distributed package), maybe using:

  "exports": {
    ".": {
      "import": {
        "types": "./types/index.d.mts",
        "default": "./dist/es/index.js"
      },
      "default": {
        "types": "./types/index.d.ts",
        "default": "./dist/cjs/index.js"
      }
    }
  },

and symlinking types/index.d.mts to types/index.d.ts is the least invasive change.

michael42 avatar May 22 '22 12:05 michael42

Would this be good enough?

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

I don't think the d.mts vs d.ts extension in the typing file changes behavior.

dzearing avatar Jun 01 '22 20:06 dzearing

The PR in https://github.com/rollup/plugins/pull/1196 should fix these I believe?

guybedford avatar Jun 01 '22 20:06 guybedford

@dzearing:

I don't think the d.mts vs d.ts extension in the typing file changes behavior.

It does (when not specifying type: "module"), at least the default export behaves differently.

@guybedford:

The PR in #1196 should fix these I believe?

Awesome, I think that would fix it, yeah.

michael42 avatar Jun 12 '22 14:06 michael42

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it.

stale[bot] avatar Aug 13 '22 04:08 stale[bot]

Cannot find module '@rollup/plugin-node-resolve'

i am runing yarn command after that facing this issue . i can not understand what is the issue

Rajnath31 avatar Nov 17 '23 11:11 Rajnath31