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

Upgrade Ava and @sindresorhus/tsconfig #2109

Closed
wants to merge 10 commits into from

Conversation

ClementValot
Copy link

Following discussions on #2051

  • Upgrade ava to V4
  • Minor packages upgrades
  • Removing tsconfig property noPropertyAccessFromIndexSignature
  • Upgrade @sindresorhus/tsconfig to 3.0.1

Checklist

  • [ X ] I have read the documentation.
  • [ X ] I have included a pull request description of my changes.

- Remove nonSemVerExperiments which is no longer experimental in V4
- Comply with ava.Macro V4 format
- Use ! operator after ava.throwsAsync which can return undefined in V4
& ((name: 'uploadProgress' | 'downloadProgress', listener: (progress: Progress) => void) => T)
/**
To enable retrying on a Got stream, it is required to have a `retry` handler attached.

When this event is emitted, you should reset the stream you were writing to and prepare the body again.

See `got.options.retry` for more information.
*/
*/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't make unrelated changes

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDE auto-indent shenanigans :(
Fixed

"target": "es2020", // Node.js 14
"lib": [
"es2020"
"moduleResolution": "Node",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Atm got 12.3.1 has moduleResolution=Node inherited from @sindresorhus/tsconfig 2.0.0

Switching to @sindresorhus/tsconfig 3.0.1 sets moduleResolution to Node16 which breaks type checking when building got (due to issues with how some packages interact with moduleReolution=Node16, see debugging attempts in #2051 (comment) )

Overriding moduleResolution to Node here allows to upgrade tsconfig without breaking the package.
It should ultimately be removed when the resolution with the problematic packages is okay.

I added a comment in the file, does that sound reasonable?

@ClementValot
Copy link
Author

I botched unit tests because ava cannot interpret typescript transpiled with module:Node16
I fixed it by adding module:ES2020 to ts-node.

Quick question @szmarczak, I see you're a maintainer of https://github.com/avajs/typescript, should it be added to this project?

Also unit test can't run properly on my setup, but there are a few that actually broke due to a FakeTimer issue, looking into it

  timeout › lookup timeout

  Rejected promise returned by test. Reason:

  TypeError {
    message: 'Can\'t install fake timers twice on the same global object.',
  }

  › Object.install (node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1642:19)
  › exec (file:///test/helpers/with-server.ts:18:38)

@sindresorhus
Copy link
Owner

Minor packages upgrades
Removing tsconfig property noPropertyAccessFromIndexSignature

These are unrelated changes and should be done in a separate pull request.

@sindresorhus
Copy link
Owner

Quick question @szmarczak, I see you're a maintainer of https://github.com/avajs/typescript, should it be added to this project?

ts-node is better in our case as it means it will not need to rewrite paths and stuff.

@sindresorhus
Copy link
Owner

Closing in favor of 8630815 and #2051 (comment). Help welcome getting the dependencies fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants