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

Generate index.d.mts and index.d.cts #238

Closed
pi0 opened this issue Mar 27, 2023 · 15 comments · Fixed by #273
Closed

Generate index.d.mts and index.d.cts #238

pi0 opened this issue Mar 27, 2023 · 15 comments · Fixed by #273

Comments

@pi0
Copy link
Member

pi0 commented Mar 27, 2023

References: #226

@timofei-iatsenko
Copy link

Can confirm this. Calling from cjs context a package with following package.json:

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

Will result in

TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@lingui/message-utils/generateMessageId")' call instead.   To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field "type": "module" to '/Users/tim/projects/js-lingui/packages/macro/package.json'.

As you see even explicit "types" condition for require/import doesn't help. It works only if i create 2 d.ts files with correct extension:

    "./generateMessageId": {
      "import": {
        "types": "./dist/generateMessageId.d.mts",
        "default": "./dist/generateMessageId.mjs"
      },
      "require": {
        "types": "./dist/generateMessageId.d.cts",
        "default": "./dist/generateMessageId.cjs"
      }
    },

If in dist folder would be a file without concrete extension, such as just d.ts it will take precedence over d.cts and d.mts and since this package has "type": "module" would be treated as module and require from cjs context would not work in typescript.

Also, what should be considered. When stubbing, d.ts file for ESM should be produced with extension in imports, otherwise it doesn't work:

// stub in /dist
export * from "/Users/tim/projects/js-lingui/packages/message-utils/src/generateMessageId"; // <-- doesn't work
export * from "/Users/tim/projects/js-lingui/packages/message-utils/src/generateMessageId.ts"; // <-- work

@karlhorky
Copy link

One thing to also consider is that "types" is not necessarily required if you have a sibling file with the correct extension:

Andrew Branch: If every index.mjs has a sibling index.d.mts and every index.cjs has a sibling index.d.cts and every index.js has a sibling index.d.ts, then nothing in package.json needs to explicitly reference the declaration files. TypeScript can always figure it out by following the implementation file config and then looking for a sibling.

@kidonng
Copy link
Contributor

kidonng commented Mar 31, 2023

@karlhorky I assume lots of tools/websites do look at that field to determine whether a package has types or not.

@kidonng
Copy link
Contributor

kidonng commented Mar 31, 2023

It seems currently we can only get ESM types with rollup builder:

await typesBuild.write({
dir: resolve(ctx.options.rootDir, ctx.options.outDir),
format: "esm",
});

@timofei-iatsenko
Copy link

Also related: microsoft/TypeScript#50466

@timofei-iatsenko
Copy link

timofei-iatsenko commented Mar 31, 2023

Generating proper cjs typings might be tricky. It will go smooth only if you don't have a default export in your library. But if you have - a real mess happened.

The problem is Rollup depending on configuration / input would generate different exports semantics for default exports

// input
export default class Foo {}

// output
class Foo {}
module.exports = Foo

If you have named + default exports or if you set output.exports = 'named'

// input
export default class Foo {}

// output
class Foo {}
module.exports.default = Foo

This led to the different typings in cjs:

// d.cts
// module.exports = Foo
// ↓ ↓ ↓ ↓ ↓ ↓
exports = Foo;

// module.exports.default = Foo
// ↓ ↓ ↓ ↓ ↓ ↓
export default Foo;

So as long you don't have any default exports in the library, you can simply copy-paste d.ts files and rename to d.cts + d.mts but if you have, you need to do an additional analysis to determine which exact semantics should be used in cjs typings.

Unfortunately, rollup-bundle-dts don't do this out of the box.

One of the solutions might be fixing Rollup's output.exports to named by default. And just copying typings.
It seems that all other tools (Typescript, Babel, Webpack) generates this kind of semantics for default exports no matter what.

Rollup REPL with default example
Rollup output.exports docs

@sadeghbarati
Copy link

sadeghbarati commented Mar 31, 2023

Hi Pooya, more context here

https://twitter.com/atcb/status/1634653474041503744?s=20

I'm so confused 😖, as far as I understand from this tweet proper package.json should look like this?

{
  "type": "module", // not required, it's based on environments, or lib author interest
  "exports": {
    ".": {
      "import": "./dist/index.mjs",
      "require": "./dist/index.cjs",
    },
    "./another": {
      "import": "./dist/another.mjs",
      "require": "./dist/another.cjs",
    },
    "main": "./dist/index.cjs", // for pre node <16?
    "module": "./dist/index.mjs", // for pre node <16?
    "types": "./dist/index.d.[mts|cts|ts]" // for pre node <16? but which one
  }
}

Or like this

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

@karlhorky
Copy link

cc @andrewbranch, since you've been mentioned a few times here

@timofei-iatsenko
Copy link

@sadeghbarati the twitter thread you referenced is very helpful. Regarding your questions:

{
 "main": "./dist/index.cjs", // for pre node <16?
 "module": "./dist/index.mjs", // for pre node <16?
 "types": "./dist/index.d.[mts|cts|ts]" // for pre node <16? but which one
}

main is for older version of node (i think pre <13 ? ). So for > 14 you don't need this. module is for older bundlers. But there is still some runtimes (Metro for example, which doesn't support this) also older bundlers such webpack 4 doesn't support exports field.

If summarize and simplify a lot, if you don't have default export in your module you just need to copy paste d.ts and rename them to d.mts and d.cts. Put them next to their's counterpart and you good to go.

If you do have a default export, look my previous messages for explanation.

@pi0 another big thing to consider. @andrewbranch made a great tool which could be incorporated as part of unbuild to validate typings https://github.com/arethetypeswrong/arethetypeswrong.github.io on a validate step.

@pi0
Copy link
Member Author

pi0 commented Apr 3, 2023

Thanks for all input, references, and ideas.

I think going forward with a simple copy/pasted version of .d.ts (targeting modules) to .d.mts and basic compatibility for CJS .d.cts assuming named exports (Defaulting output.exports to named by default seems a good idea 👍🏼) and also add back types to the exports subpaths once we have it.

I understand there are different opinions on what we should recommend for a standard package.json. Here are my goals for unjs and unbuild:

  • Mainly focus and target ESM format as a first-class option
  • Support CJS mainly to fill in the migration gap in ecosystem as long as possible (not making it a blocker requirement)
  • Explicit extensions/fields for maximum backward compatibility.

Regarding top-level fields other than exports, they are mainly for backward and tooling compatibility. I highly recommend setting them:

  • main: Resolving to CJS build because when this field is being resolved, the runtime does not have exports field support
  • module: An old convention for bundler esm format picked by bundlers
  • types: Older typescript versions do not support types field from exports. Pointing to the esm format. I believe we should introduce types into named exports too once the CJS/ESM format concerns are resolved. It allows proper and explicit subpath type mapping (even if typescript works without it)
  • type: module: Even tough we use explicit .mjs/.cjs in output, this field sets the default package format to the ESM which i think it is a safe addition.

@andrewbranch
Copy link

Note that renaming/copying declaration files is a fundamentally unchecked operation. It might result in a valid output, but it might not. I don’t know enough about what unbuild is doing to recommend something concrete. But basically every build system for packages I’ve ever seen does something like this:

  • Accept ESM inputs
  • Generate ESM and CJS outputs with a single-file transpiler
  • Check the inputs with tsc, which will only be valid for one of the output formats at most
  • Generate type declarations with tsc, which again will only be valid for one of the output formats

Now suppose you copy/rename/transform the type declarations from one output to another. Even if, by total coincidence, the result is a correct representation of the JS it claims to type, nobody ever checked if that JS was type-safe. The problem is that import { foo } from "bar" and const { foo } = require("bar") can point to completely different objects in different files with different shapes, and if you only ran type-checking once, you only checked one of two scenarios. There’s no such thing as type-safe dual emit if some kind of resolution- and type-aware system (e.g. a type checker) didn’t verify both the CJS and ESM outputs.

@xiaoxiangmoe
Copy link
Contributor

Note that renaming/copying declaration files is a fundamentally unchecked operation. It might result in a valid output, but it might not.

So we should create both index.cts and index.mts to generate valid index.d.cts and index.d.mts.

@Hebilicious
Copy link
Member

Hebilicious commented Jun 4, 2023

Trying to get a few of my libraries built with unbuild to appear as valid with https://github.com/arethetypeswrong/arethetypeswrong.github.io
and I'm getting hit by all of the issues in this thread.

Am I correct to assume that we want is for unbuild to support the following export map / fields ? :

  "type": "module",
	// ...
  "exports": {
    ".": {
      "require": {
        "types": "./dist/module.d.cts",
        "default": "./dist/module.cjs"
      },
      "import": {
        "types": "./dist/module.d.mts",
        "default": "./dist/module.mjs"
      },
      "types": "./dist/module.d.ts",
      "default": "./dist/module.mjs"
    },
    "./*": "./*"
  },
  "main": "./dist/module.cjs",
  "module": "./dist/module.mjs",
  "types": "./dist/types.d.ts",
  "files": [
    "dist",
    "*.d.ts",
    "*.cjs",
    "*.mjs"
  ],

according to @thekip we would have issues with packages that have a default export ...

For the meantime I wrote a tiny script that I run after unbuild :

import { copyFileSync } from "node:fs"
import { resolve } from "node:path"
import { fileURLToPath } from "node:url"

import fg from "fast-glob"

const dir = fileURLToPath(new URL("..", import.meta.url))

const dts = await fg(resolve(dir, "dist/**/*.d.ts"))

// Create MTS and CTS files
const dmts = dts.map(f => copyFileSync(f, f.replace(/\.d\.ts$/, ".d.mts")))
const dcts = dts.map(f => copyFileSync(f, f.replace(/\.d\.ts$/, ".d.cts")))

console.log(`Copied ${dmts.length} files from \`*.d.ts\` to \`*.d.mts\``)
console.log(`Copied ${dcts.length} files from \`*.d.ts\` to \`*.d.cts\``)

And that does the trick.
However the proper approach should be to build and run tsc for esm and cjs separately, and then to merge them with a proper export map.

@xiaoxiangmoe
Copy link
Contributor

@pi0 Hi, I create a PR to fix it.

@Hebilicious
Copy link
Member

Just discovered another tool that support this, linking here for reference
https://github.com/privatenumber/pkgroll

sentience added a commit to cultureamp/js-lingui that referenced this issue Aug 24, 2023
The recent lingui#1742 explicitly exports TypeScript type definitions
from package.json:

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

Unfortunately, [as documented in the TypeScript Handbook][handbook],
CommonJS (`require`) and ESM (`import`) module versions
cannot share the same index.d.ts file:

> It’s important to note that the CommonJS entrypoint and the ES module entrypoint each needs its own declaration file, even if the contents are the same between them. Every declaration file is interpreted either as a CommonJS module or as an ES module, based on its file extension and the "type" field of the package.json, and this detected module kind must match the module kind that Node will detect for the corresponding JavaScript file for type checking to be correct. Attempting to use a single .d.ts file to type both an ES module entrypoint and a CommonJS entrypoint will cause TypeScript to think only one of those entrypoints exists, causing compiler errors for users of the package.

I am seeing the compiler errors that this documentation warns of.

This issue appears to have been recently fixed in
the unbuild build tool that js-lingui depends upon
(see issue unjs/unbuild#238
and fix unjs/unbuild#273),
and that fix was released in [unbuild 2.0.0-rc0].

This fix for detect-locale, therefore, is to upgrade to unbuild 2.0.0,
and use the separate .d.cts and .d.mts type declaration files
it outputs.

**Note:** This moves detect-locale onto a newer major version of unbuild
than the other packages in this monorepo.
It may be preferred to upgrade them all,
but I am not familiar enough with the other packages to do this quickly.

[handbook]: https://www.typescriptlang.org/docs/handbook/esm-node.html#packagejson-exports-imports-and-self-referencing
[unbuild 2.0.0-rc0]: https://github.com/unjs/unbuild/blob/main/CHANGELOG.md#v200-rc0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants