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

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

Closed
michael42 opened this issue May 22, 2022 · 6 comments

Comments

@michael42
Copy link

michael42 commented May 22, 2022

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
Copy link
Author

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.

@dzearing
Copy link

dzearing commented Jun 1, 2022

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.

@guybedford
Copy link
Contributor

The PR in #1196 should fix these I believe?

@michael42
Copy link
Author

@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.

@stale stale bot added the x⁷ ⋅ stale label Aug 11, 2022
@stale
Copy link

stale bot commented Aug 13, 2022

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 stale bot closed this as completed Aug 13, 2022
@Rajnath31
Copy link

Rajnath31 commented Nov 17, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants