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

set declaration to be index.d.cts for cjs format #448

Open
mesqueeb opened this issue May 20, 2023 · 6 comments
Open

set declaration to be index.d.cts for cjs format #448

mesqueeb opened this issue May 20, 2023 · 6 comments
Labels
kind: feature New feature or request solution: workaround available There is a workaround available for this issue topic: TS version Related to a change in a TS version

Comments

@mesqueeb
Copy link
Contributor

currently

With this plugin I generate index.cjs via rollup and this plugin generates the index.d.ts next to it.

desired

how can I set the generated index.d.ts file to be called index.d.cts instead?

Versions

  System:
    OS: macOS 13.3.1
    CPU: (8) arm64 Apple M2
    Memory: 309.73 MB / 24.00 GB
    Shell: 3.6.0 - /opt/homebrew/bin/fish
  Binaries:
    Node: 16.20.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 8.19.4 - /usr/local/bin/npm
  npmPackages:
    rollup: ^3.22.0 => 3.22.0
    rollup-plugin-typescript2: ^0.34.1 => 0.34.1
    typescript: ^5.0.4 => 5.0.4

rollup.config.js

:
import typescript from 'rollup-plugin-typescript2'
import pkg from '../package.json' assert { type: 'json' }

export default {
  input: 'src/index.ts',
  output: [
    { file: pkg.exports['.'].require.default, format: 'cjs' },
    { file: pkg.exports['.'].import.default, format: 'esm' },
  ],
  external: Object.keys(pkg.dependencies || []),
  plugins: [typescript({ tsconfigOverride: { exclude: ['test/**/*'] } })],
}

tsconfig.json

:
{
  "compilerOptions": {
    "allowJs": false,
    "baseUrl": ".",
    "declaration": true,
    "downlevelIteration": true,
    "esModuleInterop": true,
    "isolatedModules": true,
    "lib": ["esnext", "DOM", "DOM.Iterable"],
    "module": "esnext",
    "moduleResolution": "node",
    "noImplicitReturns": true,
    "noUnusedLocals": true,
    "skipLibCheck": true,
    "sourceMap": true,
    "strict": true,
    "target": "es2019",
    "useDefineForClassFields": true
  },
  "include": ["src/**/*", "test/**/*"]
}

package.json

:
  "sideEffects": false,
  "type": "module",
  "types": "./dist/index.d.ts",
  "module": "./dist/index.js",
  "main": "./dist/index.js",
  "exports": {
    ".": {
      "require": {
        "types": "./dist/cjs/index.d.ts",
        "default": "./dist/cjs/index.cjs"
      },
      "import": {
        "types": "./dist/index.d.ts",
        "default": "./dist/index.js"
      }
    }
  },
  "files": [
    "dist"
  ],
  "engines": {
    "node": ">=12.13"
  },
@mesqueeb
Copy link
Contributor Author

current workaround:

import typescript from 'rollup-plugin-typescript2'
import pkg from '../package.json' assert { type: 'json' }
import fs from 'fs'

function renameCjsTypeDeclaration() {
  return {
    name: 'renameCjsTypeDeclaration',
    writeBundle: {
      sequential: true,
      order: 'post',
      handler({ dir, format }) {
        if (format === 'cjs') {
          fs.rename('dist/cjs/index.d.ts', 'dist/cjs/index.d.cts', console.error)
        }
      },
    },
  }
}

export default {
  input: 'src/index.ts',
  output: [
    { file: pkg.exports['.'].import.default, format: 'esm' },
    { file: pkg.exports['.'].require.default, format: 'cjs' },
  ],
  external: Object.keys(pkg.dependencies || []),
  plugins: [
    typescript({ tsconfigOverride: { exclude: ['test/**/*'] } }),
    renameCjsTypeDeclaration(),
  ],
}

@agilgur5 agilgur5 added the solution: workaround available There is a workaround available for this issue label May 20, 2023
@agilgur5 agilgur5 changed the title how can we set the output index.d.ts file to be index.d.cts for the cjs format instead? set output of index.d.ts file to be index.d.cts for the cjs format May 20, 2023
@agilgur5 agilgur5 added the kind: feature New feature or request label Jul 7, 2023
@agilgur5
Copy link
Collaborator

agilgur5 commented Jul 7, 2023

So CJS and Rollup's CJS option have been available long before .cts or .cjs. As such, any logic would have to be able to output with different file endings.

We could introduce an option to customize file endings.

Another option could be to check the file ending of the output JS file and mirror that for the typings. As in, if outputting as .cjs, then output typings as .d.cts.
That would be considered a breaking change though since it would change existing behavior.

That's a great, simple workaround in the meantime! Thanks for providing the code so that other users can do the same if needed!

@agilgur5 agilgur5 added the topic: TS version Related to a change in a TS version label Jul 7, 2023
@ezolenko
Copy link
Owner

ezolenko commented Jul 7, 2023

That would be considered a breaking change though since it would change existing behavior.

Would all the tooling that uses typings (debuggers, language servers, etc) expect correct extension though? Or do they usually have fallbacks to d.ts?

If so, changing typing extension to match output extension would break or obsolete mostly workarounds. A minor version bump either way.

@agilgur5
Copy link
Collaborator

agilgur5 commented Jul 7, 2023

I was more thinking in the opposite direction -- tools that don't yet support .cts would break. For example, if a user has a declaration processor plugin after rpt2, this change could break that user's plugin pipeline (unless they have already updated that plugin to a version that supports .cts).

Either way, it is changed behavior. And yea, since rpt2 is still 0.x, a minor is considered breaking, so that would work. Just not a patch.

@agilgur5 agilgur5 changed the title set output of index.d.ts file to be index.d.cts for the cjs format set declaration to be index.d.cts for cjs format Jul 9, 2023
@agilgur5
Copy link
Collaborator

how can I set the generated index.d.ts file to be called index.d.cts instead?

So there is potentially a small problem here. If your input is index.mts, TypeScript would only generate index.d.mts (similar for .cts).

Rollup cannot convert a declaration file to cjs format as Rollup cannot interpret TS files.
The TypeScript compiler might be able to do that when module: "commonjs", but I'm not sure it would. Either way, we only set the tsconfig once for the whole program, so being able to change this on-the-fly during Rollup's output phase may not be possible or may require some decent refactoring 😕

@ezolenko
Copy link
Owner

ezolenko commented Jul 11, 2023

Using multiple rollup configs instead of multiple outputs in one config might be needed, similar to what rpt2 does itself: https://github.com/ezolenko/rollup-plugin-typescript2/blob/master/rollup.config.self.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: feature New feature or request solution: workaround available There is a workaround available for this issue topic: TS version Related to a change in a TS version
Projects
None yet
Development

No branches or pull requests

3 participants