unjs / mkdist

Lightweight file-to-file transpiler.
MIT License
363 stars 25 forks source link

Declarations not genered to `.mts` and `.cts` files #138

Open brawaru opened 1 year ago

brawaru commented 1 year ago

Environment

mkdist 1.1.2 on Node.js 16.14.2, v18.14.0.

Reproduction

  1. Have .mts and .cts files in your src directory.
  2. Run mkdist -d --ext js
Example project files ```ts // farewells.cts export function sayGoodbye(name: string) { console.log(`Goodbye, ${name}!`) } ``` ```ts // greetings.ts export function sayHello(name: string) { console.log(`Hello, ${name}!`) } ``` ```ts // index.mts import { sayHello } from './greetings.js' import { sayGoodbye } from './farewells.cjs' sayHello('World') sayGoodbye('World') ```

StackBlitz example: https://stackblitz.com/edit/node-erv3qh?file=package.json&view=editor (use pnpm build:tsc to build with TypeScript and pnpm build:mkdist to build with mkdist, start to run ./dist/index.mjs with Node).

Describe the bug

mkdist ignores .cts and .mts files and copies them as is instead of compiling them to .cjs (+.d.cts) and .mjs (+.d.mts) respectfully.

Additional context

I could've tried to explain the importance of these file extensions on my own but Bing did a nicer job than me 😢

The .mjs and .cjs extensions are used to specify the module type of a JavaScript file in Node.js 16+. The .mjs extension indicates that the file is an ECMAScript module (ESM), which is a standard format for modular JavaScript code. The .cjs extension indicates that the file is a CommonJS module, which is a legacy format that was widely used before ESM was introduced^1.

The .mts and .cts extensions are used to specify the module type of a TypeScript file. TypeScript is a superset of JavaScript that adds static types and other features. The .mts extension indicates that the file is an ESM TypeScript file, which will be emitted as an .mjs file when compiled to JavaScript. The .cts extension indicates that the file is a CommonJS TypeScript file, which will be emitted as a .cjs file when compiled to JavaScript^1.

These extensions help Node.js to determine how to load and execute different types of modules without relying on ambiguous heuristics or configuration options^3. They also help developers to write clear and consistent code across different environments.

While this is a nice explanation, it fails to mention a few other important things:

If your file has a .ts extension, TypeScript always assumes the resulting file will have .js extension. So if any of the files in your project imports the other file that has .ts extension, and you use Node16 (NodeNext) module resolution (which you very much should btw), TypeScript forces you to write import with .js extension, which is incompatible with mkdist, unless you force it in .js extension mode (--ext js), which you cannot do in environments like nuxt-module-builder without patching it (which is exactly what I'm struggling with and what I am doing).

The other important thing is that .ts, .cts, .mts on the declaration files imply the existence of the file with the same basename: that is, index.d.ts says ‘the declaration file is for index.js’, a.d.cts is ‘for a.cjs’, and b.d.mts is ‘for b.mjs’. That becomes super important for libraries that ship with wildcard exports in package.json, as well helps you as developer to avoid writing types condition which may or may not lead to a correct declaration file (without types export condition TypeScript looks for [basename].d.[ts/cts/mts]).

{
  "exports": {
    "./locale-data/*": { "import": "./dist/locale-data/*" }
  }
}

Logs

No response

pi0 commented 1 year ago

Hi and thanks for well explaining in issue. Primary support added vis #162. Please feel free to mention if you think something is not following up spec with new handler.

brawaru commented 1 year ago

@pi0 I have quickly checked the 1.3.0 and it doesn't seem to produce valid output in the updated playground: https://stackblitz.com/edit/node-yk5rfx?file=package.json&view=editor :(

In the playground the output of mkdist should match the TypeScript output (pnpm build:tsc):

Currently it outputs (pnpm build:mkdist):

The test is also indeed flawed.

To simplify:

While updating I noticed I used ext argument (which I removed in updated playground), and I'm not sure how ext argument can be applied in this scenario. Maybe it can affect only .js/.ts source files and override tsConfig.compilerOptions.module / pkgJson.type, e.g. if you set it to mjs, then all .js source files will be emitted as .mjs and have associated .d.mts declarations, whereas .cjs and .mjs files are emitted as is.