tsdjs / tsd

Check TypeScript type definitions
MIT License
2.39k stars 68 forks source link

Move to ESM, remove `index.d.ts` requirement #186

Open tommy-mitchell opened 1 year ago

tommy-mitchell commented 1 year ago

Closes #183. Closes #191. Closes #175.

Checklist:

We should also update the assertion definitions to use const type parameters.


Speaking of tests, trying to run AVA causes errors with @tsd/typescript:

npx ava source/test/test.ts

(node:69218) ExperimentalWarning: --experimental-loader is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
  Uncaught exception in source/test/test.ts

  SyntaxError: The requested module '@tsd/typescript' does not provide an export named 'TypeFormatFlags'

  ✘ source/test/test.ts exited with a non-zero exit code: 1
  ─

  1 uncaught exception

In addition, updating AVA means that t.throwsAsync can also return undefined:

test('failure', async t => {
    const cwd = path.join(__dirname, 'fixtures/failure');
    const {exitCode} = await t.throwsAsync<ExecaError>(execa('../../../cli.js', {cwd}));
    //=> Property 'exitCode' does not exist on type 'ExecaError | undefined'.

    t.is(exitCode, 1);
});

cc: @sindresorhus

tommy-mitchell commented 1 year ago

Using tsx would resolve #175 as well.

mrazauskas commented 1 year ago

By the way, this is 100% breaking change for programmatic API users. Perhaps #75 could be considered? Would be enough to return an object. This would make it easier to add any additional props in the future.

The #75 was needed for jest-runner-tsd, but it was rather unclear if that will ever happen. I fork tsd and released tsd-lite. You might be interested to look around:

sindresorhus commented 1 year ago

What's the benefit of using .tsx? It's not clear to me.

sindresorhus commented 1 year ago

I think what you want is this: https://devblogs.microsoft.com/typescript/announcing-typescript-5-0-beta/#allowimportingtsextensions

sindresorhus commented 1 year ago

SyntaxError: The requested module '@tsd/typescript' does not provide an export named 'TypeFormatFlags'

The ES module resolution is stricter about exports. Make sure it's actually exported correctly.

tommy-mitchell commented 1 year ago

What's the benefit of using .tsx? It's not clear to me.

tsx is a tool like ts-node for transpiling TS in-memory. The name comes from modeling npx. It’s faster than ts-node and works better with ESM, but skips type checking.

The naming is unfortunate.

https://github.com/avajs/ava/issues/2593#issuecomment-1345069993

sindresorhus commented 1 year ago

tsx is a tool like ts-node for transpiling TS in-memory. The name comes from modeling npx. It’s faster than ts-node and works better with ESM, but skips type checking.

Ok. We definitely want type checking to ensure they are valid. But we could also just run tsc --noEmit before to type check.

sindresorhus commented 1 year ago

Let me know if anything is blocked on me commenting on something.

tommy-mitchell commented 1 year ago

But we could also just run tsc --noEmit before to type check.

I decided to go with that as well. I've been doing more locally, you're fine.

SyntaxError: The requested module '@tsd/typescript' does not provide an export named 'TypeFormatFlags'

TypeFormatFlags is an enum exported bytypescript itself. I'm going to see if using ts-node works instead.


Re: #75, should we close that PR and open a new one? It's definitely stale, and the diff is pretty extraneous at this point. I can open a reminder issue.

sindresorhus commented 1 year ago

Re: https://github.com/SamVerschueren/tsd/pull/75, should we close that PR and open a new one? It's definitely stale, and the diff is pretty extraneous at this point. I can open a reminder issue.

Yes

tommy-mitchell commented 1 year ago

Seems like the issue with the tests is that they are being transpiled to CJS, but since "type": "module" is set the tests throw.

Related, is there a reason that we have tsconfig.tsd.json and not tsconfig.json?

sindresorhus commented 1 year ago

Related, is there a reason that we have tsconfig.tsd.json and not tsconfig.json?

https://github.com/SamVerschueren/tsd/pull/100

tommy-mitchell commented 1 year ago

I've uploaded some more changes (tests are still failing). I'll resolve the merge conflicts.

A couple highlights: moved to xo for linting, updated the test script, and consolidated the tests via macros (still need to do so for the CLI tests).

tommy-mitchell commented 1 year ago

SyntaxError: The requested module '@tsd/typescript' does not provide an export named 'TypeFormatFlags'

The ES module resolution is stricter about exports. Make sure it's actually exported correctly.

Turns out this is because the TypeScript compiler is CJS: https://github.com/microsoft/TypeScript/issues/32949

sindresorhus commented 1 year ago

@tommy-mitchell Just a reminder that this is still open. If you're simply busy, feel free to ignore this.

tommy-mitchell commented 1 year ago

@sindresorhus this is on my todo list, I’ve got some local changes that are unpushed. If you have some time, I’d love your feedback on #196

tommy-mitchell commented 7 months ago

Ended up removing the index.d.ts requirement in this PR because it was giving me trouble with tests. Merged the changes from main. Next steps:

tommy-mitchell commented 7 months ago

Are we still supporting testing types in CJS packages after moving to ESM?

tommy-mitchell commented 7 months ago

Seems like xo is failing because it can't find a tsconfig.json, even though I set it in parserOptions.project:

file:///home/runner/work/tsd/tsd/node_modules/xo/lib/options-manager.js:163
        options.tsConfig = tsConfigResolvePaths(getTsconfig(options.tsConfigPath).config, options.tsConfigPath);
                                                                                 ^

TypeError: Cannot read properties of null (reading 'config')
    at handleTSConfig (file:///home/runner/work/tsd/tsd/node_modules/xo/lib/options-manager.js:163:76)
    at mergeWithFileConfig (file:///home/runner/work/tsd/tsd/node_modules/xo/lib/options-manager.js:137:19)
    at async parseOptions (file:///home/runner/work/tsd/tsd/node_modules/xo/lib/options-manager.js:578:51)
    at async Promise.all (index 0)
    at async getOptionGroups (file:///home/runner/work/tsd/tsd/node_modules/xo/lib/options-manager.js:594:21)
    at async Object.lintFiles (file:///home/runner/work/tsd/tsd/node_modules/xo/index.js:80:17)
    at async file:///home/runner/work/tsd/tsd/node_modules/xo/cli.js:212:18
tommy-mitchell commented 7 months ago

Seems like there should be a way for us to just tsconfig.json directly, I'll try to see if I can do that without running into the issues from #100.

sindresorhus commented 7 months ago

Are we still supporting testing types in CJS packages after moving to ESM?

No