Fix TS 4.7 compatibility

[RebeccaStevens]

See TS 4.7-rc Release Notes for details.

[novemberborn]

@RebeccaStevens is this a required change for TypeScript 4.7 / Node.js 16 compatibility?

I do want to use this feature to distribute ESM and CJS type definitions. I was thinking we'd look at that when 4.7 ships as part of AVA 5. (We can drop Node.js 12 support anyway.)

[RebeccaStevens]

From my testing so far, it seems to be required.

I did read somewhere that top level exports should still grab the top level "types" field from package.json if there isn't a more specific one defined in the "exports" field. But from my testing this doesn't seem to be the case at the moment. (Maybe it will be fixed when 4.7 stable is released).

[RebeccaStevens]

See this comment for why I added .d.cts files.
microsoft/TypeScript#46408 (comment)

Edit: This doesn't work as I expected. I'll probably revert the last commit but it seem like a bad idea to dupe all the type files. Maybe a build step would be wanted to this?

Alternatively, it seem that if the .js file extension is used instead of .cjs for the CJS, both ESM and CJS can use the same .d.ts file.

[novemberborn]

What does this imply? Can we model the module.exports = test behavior, as well as exporting the types?

[RebeccaStevens]

Hey, I've gotten a bit better of an idea now as to how this new typing thing works. I'll update this pr soon to improve things.

[RebeccaStevens]

@novemberborn From my testing, this latest push I made seems to be working properly.

Will want to wait for a reply to this comment first before proceeding though.

[RebeccaStevens]

Seems like everything is good to go.

[novemberborn]

@RebeccaStevens what do you think about making TS 4.7 the minimum version in AVA 5? I think that would make it possible to include some tests for these new file formats, no?

I've tried some local files but I can't get test.cts to compile:

const test = require('ava')

test('foo', t => t.pass())

{
  "compilerOptions": {
    "target": "es2022",
    "moduleResolution": "node16",
    "strict": true,
  }
}

test.cts:3:13 - error TS7006: Parameter 't' implicitly has an 'any' type.

3 test('foo', t => t.pass())
              ~


Found 1 error in test.cts:3

[RebeccaStevens]

So in .cts files, it seems that you need to use import test = require("ava") to get the right types.
In .cjs files, const test = require("ava") should work but it seems like it's loading the esm types instead of the cjs types. I'm not sure why this is, it might be a TypeScript bug.

I haven't been following Ava 5's development so I can't really comment on it requiring TS 4.7+. However, it should still be possible to write tests for these new types without this requirement; the tests would just have to run in TS 4.7 environment.

[novemberborn]

So in .cts files, it seems that you need to use import test = require("ava") to get the right types.

That's… surprising. Does const test = require("ava") work in .ts files targeting CommonJS?

In .cjs files, const test = require("ava") should work but it seems like it's loading the esm types instead of the cjs types. I'm not sure why this is, it might be a TypeScript bug.

😞

I haven't been following Ava 5's development so I can't really comment on it requiring TS 4.7+. However, it should still be possible to write tests for these new types without this requirement; the tests would just have to run in TS 4.7 environment.

There isn't any development as such. Dropping Node.js 12 requires a major bump, so once this is in AVA 4 it could be nice to upgrade to TS 4.7 as the recommended/tested setup.

I've added the 4.7 RC to the test matrix.

[RebeccaStevens]

@novemberborn TS 4.7 has launched. Is there any blockers left for this?

Also, with regards to Ava 5, I think it would be a good idea to up level all the code to esm and then down level to cjs with a build tool. The current approach of just defining esm that imports the cjs code can lead to a few issues. I could look into creating a PR for this if you like. Has any work been done specifically for Ava 5 yet?

[RebeccaStevens]

microsoft/TypeScript#49299

[novemberborn]

The current approach of just defining esm that imports the cjs code can lead to a few issues.

Could you elaborate?

Has any work been done specifically for Ava 5 yet?

No, it's just that removing Node.js 12 from the test matrix requires a major version bump, so we have an opportunity to set some new baselines.

[novemberborn]

I can't figure this out.

{
  "name": "tmp.78i8xvt0sd",
  "version": "1.0.0",
  "devDependencies": {
    "@sindresorhus/tsconfig": "^3.0.1",
    "@types/node": "^16.11.36",
    "ava": "github:RebeccaStevens/ava#node16-types-exports",
    "typescript": "^4.7.2"
  }
}

{
  "extends": "@sindresorhus/tsconfig"
}

But for test.ts and test.cts:

const test = require('ava')

test('pass', t => t.pass())

I get:

> npx tsc --noEmit
test.cts:3:14 - error TS7006: Parameter 't' implicitly has an 'any' type.

3 test('pass', t => t.pass())
               ~

test.ts:3:14 - error TS7006: Parameter 't' implicitly has an 'any' type.

3 test('pass', t => t.pass())
               ~


Found 2 errors in 2 files.

Errors  Files
     1  test.cts:3
     1  test.ts:3

It's fine for ESM, but CommonJS doesn't seem to be working with TS 4.7.

[novemberborn]

Changing to import test = require('ava') makes it worse:

test.ts:1:23 - error TS7016: Could not find a declaration file for module 'ava'. '/avajs/ava/entrypoints/main.cjs' implicitly has an 'any' type.
  If the 'ava' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module 'ava';`

1 import test = require('ava')

[novemberborn]

@sindresorhus what's your understanding of all this?

I've also tried with the following main.d.cts but it makes no difference:

import type {TestFn} from './test-fn.js';

/** Call to declare a test, or chain to declare hooks or test modifiers */
declare const test: TestFn;

/** Call to declare a test, or chain to declare hooks or test modifiers */
export = test;

[RebeccaStevens]

@novemberborn It looks like my attempt to reuse code has failed and we're going to need to duplicate the type files :(

Oh well. I'll push a fix soon.

[RebeccaStevens]

@novemberborn Push a fix, though I may have undone some of your commits. Feel free to remake them.

[RebeccaStevens]

With typescript@next the following is valid and thus wouldn't require duplicated the type files.

import type TestFn from "../index.js" assert { "resolution-mode": "import" };

declare const test: typeof TestFn;
export = test;

[RebeccaStevens]

With TypeScript 4.8 we can go back to reusing definitions?

I think so but we'll have to wait and see.

[RebeccaStevens]

It is my understanding this is just how TS 4.7 is meant to behave?

Yes, I believe so.

[RebeccaStevens]

The current approach of just defining esm that imports the cjs code can lead to a few issues.

Could you elaborate?

main.mjs imports main.cjs:

export {default} from '../lib/worker/main.cjs';

So some of the issue we are experiencing with this PR can happen with the actually JavaScript when mixing esm and cjs. There's an issue open in chai which uses this same approach.
Additionally, in environments that only support esm and not cjs, everything will fail.

[novemberborn]

Everything is fine but VScode is still using TS 4.6 be default. If you tell VScode to use the workspace version of TS then problem goes away.

It's not working with npx tsc though:

test.ts:2:14 - error TS2305: Module '"ava"' has no exported member 'Macro'.

2 import type {Macro} from 'ava'
               ~~~~~

[novemberborn]

The current approach of just defining esm that imports the cjs code can lead to a few issues.

Could you elaborate?

main.mjs imports main.cjs:

export {default} from '../lib/worker/main.cjs';

So some of the issue we are experiencing with this PR can happen with the actually JavaScript when mixing esm and cjs. There's an issue open in chai which uses this same approach. Additionally, in environments that only support esm and not cjs, everything will fail.

We're not considering any tools that compile the AVA code. This all seems to be working fine given that we have CJS and ESM entrypoints.

[RebeccaStevens]

Everything is fine but VScode is still using TS 4.6 be default. If you tell VScode to use the workspace version of TS then problem goes away.

It's not working with npx tsc though:

test.ts:2:14 - error TS2305: Module '"ava"' has no exported member 'Macro'.

2 import type {Macro} from 'ava'
               ~~~~~

It's working for me. Try ./node_modules/.bin/tsc; you might have an older version of TS install globally that npx tsc is referring to.

[RebeccaStevens]

The current approach of just defining esm that imports the cjs code can lead to a few issues.

Could you elaborate?

main.mjs imports main.cjs:

export {default} from '../lib/worker/main.cjs';

So some of the issue we are experiencing with this PR can happen with the actually JavaScript when mixing esm and cjs. There's an issue open in chai which uses this same approach. Additionally, in environments that only support esm and not cjs, everything will fail.

We're not considering any tools that compile the AVA code. This all seems to be working fine given that we have CJS and ESM entrypoints.

It works fine at a surface level but it's kind of fragile.

[novemberborn]

I think the type exports are only missing for the CJS files. Which makes sense because they're not exported there.

I'm not sure how to combine a type export with the export = syntax though.

[RebeccaStevens]

I think the type exports are only missing for the CJS files. Which makes sense because they're not exported there.

I'm not sure how to combine a type export with the export = syntax though.

Same. As far as I know, it's not possible but maybe some one else can find a solution.

In TS 4.8, the types will be able to be imported with a resolution mode assertion, thus allowing the types to be used in a cjs environment. But ideally we'd like to work without needing this.

[novemberborn]

OK, so then we can't use export = test. With an export default test you can do import ava = require('ava') and ava.default() in *.cts files. Which isn't ideal but let's blame TS 4.7.

A CJS target, using ESM syntax, works fine. But I suspect we then don't need any separate files.

[novemberborn]

But I suspect we then don't need any separate files.

Hold that thought…

[novemberborn]

@RebeccaStevens see my commits. Does this work all right for you?

[novemberborn]

It's probably best to think about the import and export stuff as TypeScript syntax, which is then emitted into JavaScript based on the configuration. And it is still the recommended syntax, even in .d.cts files, except for .cts files which are just weird.

[RebeccaStevens]

I believe with allowSyntheticDefaultImports, .cts files can use import test from "Ava".

[RebeccaStevens]

@RebeccaStevens see my commits. Does this work all right for you?

No, I'm getting errors in cjs context.

[novemberborn]

I believe with allowSyntheticDefaultImports, .cts files can use import test from "Ava".

Ah yes that seems to work.

No, I'm getting errors in cjs context.

Could you share the specific syntax? My local test files seem fine 😕

[RebeccaStevens]

Here's some screenshots of how things appear on my end.

[RebeccaStevens]

We should be able to resolve the missing types in cjs based on this comment: microsoft/TypeScript#49299 (comment)

[novemberborn]

We should be able to resolve the missing types in cjs based on this comment: microsoft/TypeScript#49299 (comment)

Aha! Let's try that.

Your CTS failure is because with my changes that now has to be import ava = require('ava') with ava.default(), which is super awkward.

[novemberborn]

We should be able to resolve the missing types in cjs based on this comment: microsoft/TypeScript#49299 (comment)

Aha! Let's try that.

Except… the test function implements an interface. I'm not sure we can merge that with a namespace.

[novemberborn]

OK so we can do export const after: AfterFn etc, so that sort of works, except you can then also import type {after}.

[novemberborn]

I'm inclined to leave it like this. We can look at exporting test, serial and maybe some others as members in a future release which should improve ergonomics.

[novemberborn]

Thanks for your help with this @RebeccaStevens!