docs: add types to default exports

[kidonng]

This is required for TypeScript to work under "module": "node16", see https://github.com//microsoft/TypeScript/issues/47792.

[karlhorky]

Hm, are you sure about this change? @andrewbranch mentioned in this comment that this is only possible if the .d.ts file can express ESM and CJS - which seems to be often not the case:

That is right, assuming a single index.d.ts can accurately describe the ESM and CJS interfaces simultaneously.

Eg. see this similar change from vue-router, which seems to cause an issue with the types:

• fix(types): adds ESM export of types vuejs/router#1457 (comment)

Or this solution that I came up with for @vitejs/plugin-react, which is kind of the opposite of what you're proposing here:

For me what fixed it was creating a patch using pnpm patch (or patch-package for Yarn / npm) with the following changes:

1. Edit package.json to remove the "types" line inside "exports" -> "."
2. Copy dist/index.d.ts to dist/index.d.mts

• ts error when moduleResolution is "node16" vitejs/vite-plugin-react#104 (comment)

And @vitejs/plugin-react depends also on unbuild 😯

Maybe @andrewbranch can weigh in here on what the best approach is?

Hopefully I'm not the one who's wrong here! If I am, sorry for the noise!

[andrewbranch]

That is right, assuming a single index.d.ts can accurately describe the ESM and CJS interfaces simultaneously.

I was wrong when I said this. Wishful thinking—a single index.d.ts file can never accurately represent a CJS and ESM file at the same time. Consequently, this change doesn’t look right. The existing config is fine, assuming ./dist/index.d.mts and ./dist/index.d.cts exist.

[pi0]

/cc @danielroe

[pi0]

We are doing this by default for new UNJS packages and in template, considering types are targeting esm first. (any ideas more than welcome to not advice using
types field)

[karlhorky]

@pi0 from my understanding of the conversation above, a single exports['.'].types value of index.d.ts for dual-published ESM / CJS packages will be problematic, and this PR should not have been merged.

Using a single exports['.'].types for dual-published packages will lead to "Masquerading as..." problems similar to the one that you can see here on Are the types wrong?:

Imports of the package under the node16 module resolution setting when the importing module is ESM (its extension is .mts or .mjs, or it has a .ts or .js extension and is in scope of a package.json that contains "type": "module") resolved to CJS types, but ESM implementations.

[karlhorky]

any ideas more than welcome to not advice using types field

Right, that's what Andrew's comment above is saying, if I'm understanding correctly - *do not* use the exports['.'].types field.

The existing config is fine, assuming ./dist/index.d.mts and ./dist/index.d.cts exist.

What I understand from this comment is that exports['.'].types does not need to be configured at all (because it was not configured in the existing config before this PR was merged) - as long as you publish your ESM + CommonJS types as ./dist/index.d.mts and ./dist/index.d.cts

[pi0]

Okay, reverted the suggestion for now until can do more research on this. Just saying rest of unjs packages (like h3) already adopted this kind of types field export and that are main target is ESM with best possible CJS support. Let's continue discussion in #238 if we can generate proper types for CJS/ESM as alternative or worth to add back this to docs.

[kidonng]

Sorry, missed the discussion here. Very informative and helpful. Should have done more research before making this change 😅

Now I need to make up for some of my previous similar PRs to other projects... but I do think if tools are currently lacking, "masquerading" is better than no types at all. (Not saying it's better than correct types.)

[karlhorky]

"Masquerading" is probably too soft of a word. If it is marked as "Masquerading ..." it is broken. Not any better than missing types.