fix(plugin-legacy): ensure correct typing for node esm #13892
Conversation
|
|
| @@ -17,7 +17,6 @@ | |||
| "types": "./dist/index.d.ts", | |||
| "exports": { | |||
| ".": { | |||
| "types": "./dist/index.d.ts", | |||
There was a problem hiding this comment.
What is the reasoning behind removing this entry? We have the same setup on all other plugins: https://github.com/vitejs/vite-plugin-vue/blob/93c444cb99d64c65b71050d0f6a5e3016f7046a1/packages/plugin-vue/package.json#L14
There was a problem hiding this comment.
TypeScript is able to pick up the adjacent .ts by removing this, .e.g ./dist/index.mjs -> ./dist/index.d.mts. What this PR does is that it emits two dts files for the two formats :(
There was a problem hiding this comment.
Because of .d.mts and .d.cts, removing it will allow TS to find correct one by itself
There was a problem hiding this comment.
@patak-dev Here is also another PR https://github.com/vitejs/vite-plugin-vue/pull/179/files
There was a problem hiding this comment.
According to https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-7.html#packagejson-exports-imports-and-self-referencing , the only way to correct provide types for both Node ESM and CJS is to have two separate declaration files, so we need to generate index.d.cts and index.d.mts.
unbuild@2 could generate index.d.cts and index.d.mts standing for types of index.cjs and index.mjs, and could generate index.d.ts for legacy typescript usage.
releated commit:
vuejs/core@d621d4c
releated PRs:
unjs/unbuild#273
vitejs/vite-plugin-vue#179
There was a problem hiding this comment.
I think this PR is good for plugins given our current setup. We could release it in a minor for the plugin. I added it to the 5.0 milestone (that we're going to start working on soon) because there is a parallel discussion about moving vite core and the plugins to ESM only. I still don't know if Vite 5 is the right major to do this, so if we don't move forward with that, we should merge this PR instead.
Polls that @bluwy started for reference:
|
As much as I dislike this TS shenanigans, I'm ok to push this for the next plugin-legacy major for now, and drop CJS in the next-next major. I'm not sure if there's any migration paths we need to implement but I think we can cut right clean then. @xiaoxiangmoe would you be able to upgrade unbuild to v2 stable and rebase the PR? |
bluwy
added
plugin: legacy
p2-nice-to-have
Description
According to https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-7.html#packagejson-exports-imports-and-self-referencing , the only way to correct provide types for both Node ESM and CJS is to have two separate declaration files, so we need to generate index.d.cts and index.d.mts.
unbuild@2 could generate index.d.cts and index.d.mts standing for types of index.cjs and index.mjs, and could generate index.d.ts for legacy typescript usage.
releated commit:
vuejs/core@d621d4c
releated PRs:
unjs/unbuild#273
vitejs/vite-plugin-vue#179
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).