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

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

Open
brawaru opened this issue Mar 15, 2023 · 2 comments · Fixed by #162
Open

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

brawaru opened this issue Mar 15, 2023 · 2 comments · Fixed by #162
Labels
bug Something isn't working

Comments

@brawaru
Copy link

brawaru commented Mar 15, 2023

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
// farewells.cts
export function sayGoodbye(name: string) {
  console.log(`Goodbye, ${name}!`)
}
// greetings.ts
export function sayHello(name: string) {
  console.log(`Hello, ${name}!`)
}
// 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 introduced12.

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

These extensions help Node.js to determine how to load and execute different types of modules without relying on ambiguous heuristics or configuration options3. 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

Footnotes

  1. https://www.typescriptlang.org/docs/handbook/esm-node.html 2

  2. https://blog.logrocket.com/how-to-use-ecmascript-modules-with-node-js/

  3. https://nodejs.org/api/packages.html

@danielroe danielroe added the bug Something isn't working label Mar 15, 2023
@pi0 pi0 closed this as completed in #162 Jul 18, 2023
@pi0
Copy link
Member

pi0 commented Jul 18, 2023

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

brawaru commented Jul 19, 2023

@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):

  • farewells.d.cts — DTS for farewells.cjs
  • farewells.cjs — CJS module
  • greetings.d.ts— DTS for greetings.js
  • greetings.js — CJS/ESM module depending on tsConfig.compilerOptions.module / pkgJson.type?
  • index.d.mts — DTS for index.mjs
  • index.mjs — ESM module

Currently it outputs (pnpm build:mkdist):

  • falewells.d.cts — DTS for farewells.cjs (file missing)
  • farewells.mjs — ESM module (no declaration)
  • greetings.d.ts — DTS for greetings.js (file missing)
  • greetings.mjs — ESM module (no declaration)
  • index.d.mts — DTS for index.mjs
  • index.mjs — ESM module (contains unresolved imports)

The test is also indeed flawed.

To simplify:

  • [filename].d.ts files tell TypeScript ‘hey, there's [filename].js, I'm a declaration for it’. Likewise, [filename].d.mts is ‘there's [filename].mjs’, and [filename].d.cts is ‘there's [filename].cjs’.

  • .mts and .cts in sources tell TypeScript how to compile these files regardless the config, .mts compiles to ESM files with .mjs extension, and .cts to CJS with .cjs.

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.

@pi0 pi0 reopened this Jul 19, 2023
@pi0 pi0 changed the title .mts and .cts copied as is Declarations not genered to .mts and .cts files Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants