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

[Feature Request] Allow opting out of ts-node #2420

Merged

Conversation

ds300
Copy link
Contributor

@ds300 ds300 commented Jan 18, 2024

What Changed

I added an option to prevent auto from installing the ts-node require hook.

Why

We recently upgraded typescript to v5 and one of our deploy scripts that used the Auto class (as opposed to the auto cli) started failing with the following error:

/home/runner/work/tldraw/tldraw/node_modules/@auto-it/core/node_modules/ts-node/dist/index.js:3
Object.defineProperty(exports, "__esModule", { value: true });
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       ^
TSError: ⨯ Unable to compile TypeScript:
error TS5109: Option 'moduleResolution' must be set to 'NodeNext' (or left unspecified) when option 'module' is set to 'NodeNext'.

    at createTSError (/home/runner/work/tldraw/tldraw/node_modules/@auto-it/core/node_modules/ts-node/dist/index.js:3:6696)
    at reportTSError (/home/runner/work/tldraw/tldraw/node_modules/@auto-it/core/node_modules/ts-node/dist/index.js:3:6848)
    at /home/runner/work/tldraw/tldraw/node_modules/@auto-it/core/node_modules/ts-node/src/index.ts:1:67
    at Object.compile (/home/runner/work/tldraw/tldraw/node_modules/@auto-it/core/node_modules/ts-node/src/index.ts:1:67)
    at Module.m._compile (/home/runner/work/tldraw/tldraw/node_modules/@auto-it/core/node_modules/ts-node/src/index.ts:1:67)
    at j (/home/runner/work/tldraw/tldraw/node_modules/tsx/dist/cjs/index.cjs:1:[11](https://github.com/tldraw/tldraw/actions/runs/7557438453/job/20576664393#step:8:12)97)
    at Object.require.extensions.<computed> [as .ts] (/home/runner/work/tldraw/tldraw/node_modules/@auto-it/core/node_modules/ts-node/src/index.ts:1:67)
    at Module.load (node:internal/modules/cjs/loader:1119:32)
    at Function.Module._load (node:internal/modules/cjs/loader:960:[12](https://github.com/tldraw/tldraw/actions/runs/7557438453/job/20576664393#step:8:13))
    at Module.require (node:internal/modules/cjs/loader:1[14](https://github.com/tldraw/tldraw/actions/runs/7557438453/job/20576664393#step:8:15)3:[19](https://github.com/tldraw/tldraw/actions/runs/7557438453/job/20576664393#step:8:20)) {
  diagnosticCodes: [ 5109 ]
}

This is most likely a problem with ts-node, since it seems to be passing an incorrect config to the ts compiler when transpiling modules. But also we already use tsx for running this particular script, so we don't even need the ts-node hook to be registered. I'd appreciate if there was a semi-official way we could opt out of ts-node when using Auto as an api rather than a cli tool, hence this PR.

Todo:

  • Add tests
  • Add docs

Change Type

Indicate the type of change your pull request is:

  • documentation
  • patch
  • minor
  • major

Copy link

codecov bot commented Jan 18, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7a6f0ae) 85.92% compared to head (edf02b6) 80.58%.

Files Patch % Lines
packages/core/src/auto.ts 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2420      +/-   ##
==========================================
- Coverage   85.92%   80.58%   -5.34%     
==========================================
  Files          69       69              
  Lines        5675     5676       +1     
  Branches     1332     1275      -57     
==========================================
- Hits         4876     4574     -302     
- Misses        718      719       +1     
- Partials       81      383     +302     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hipstersmoothie
Copy link
Collaborator

@ds300 could you add a reference to this in the docs?

@ds300
Copy link
Contributor Author

ds300 commented Feb 23, 2024

@hipstersmoothie happy to. I don't see a convenient place to mention it, can you point me in the right direction?

@hipstersmoothie
Copy link
Collaborator

So looking at the docs we don't list global options anywhere 💀 Gonna have to fix that

In the meantime:

The only way to use this option right now would be by using auto programmatically which you probably don't want. We should add this ass a global CLI option too.

  1. Add the option to defaultOptions in parse-args.ts
  2. In another PR I'll add docs for the global CLI options

@ds300
Copy link
Contributor Author

ds300 commented Feb 23, 2024

The only way to use this option right now would be by using auto programmatically which you probably don't want.

On the contrary! The only reason we need this option is because we are using auto programatically (is this kosher?). I don't think it would be an issue for anybody running auto on the cli.

@hipstersmoothie
Copy link
Collaborator

hipstersmoothie commented Feb 23, 2024

Oh wow I didn't even realize it was you lol

Totally kosher to use pragmatically like that. Given you reasoning that seems fine to me!

@hipstersmoothie hipstersmoothie added the patch Increment the patch version when merged label Feb 23, 2024
@hipstersmoothie hipstersmoothie merged commit 6774575 into intuit:main Feb 23, 2024
5 of 9 checks passed
@hipstersmoothie hipstersmoothie added minor Increment the minor version when merged and removed patch Increment the patch version when merged labels Feb 23, 2024
Copy link

🚀 PR was released in v11.1.0 🚀

@github-actions github-actions bot added the released This issue/pull request has been released. label Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants