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

[Spike] Investigate migrating off of ts-node #30426

Closed
ryanthemanuel opened this issue Oct 20, 2024 · 7 comments · May be fixed by #31131
Closed

[Spike] Investigate migrating off of ts-node #30426

ryanthemanuel opened this issue Oct 20, 2024 · 7 comments · May be fixed by #31131
Assignees
Labels
type: chore Work is required w/ no deliverable to end user

Comments

@ryanthemanuel
Copy link
Collaborator

What would you like?

Determine an alternative to ts-node.

Why is this needed?

In looking at ts-node, it appears that there has not been a lot of recent development. During the upgrade to electron 32 (and underlying Node version bump to 20+), I had to work around some active issues in ts-node. It seems like it might make maintenance easier going forward if we can find an alternative (tsx for example).

Other

@AtofStryker had done a little investigation to swapping to tsx on this branch. This could be used as a starting point.

@jennifer-shehane jennifer-shehane added the type: chore Work is required w/ no deliverable to end user label Oct 21, 2024
@MikeMcC399
Copy link
Contributor

@ryanthemanuel

ts-node is causing a deprecation message when Cypress is run in the current Node.js v22.11.0 LTS version:

(node:7665) [DEP0180] DeprecationWarning: fs.Stats constructor is deprecated.

@AtofStryker
Copy link
Contributor

likely related #30954 and #30925

@AtofStryker AtofStryker linked a pull request Feb 25, 2025 that will close this issue
3 tasks
@AtofStryker AtofStryker self-assigned this Feb 25, 2025
@AtofStryker
Copy link
Contributor

AtofStryker commented Feb 26, 2025

tsx

I did some spiking into using tsx as a replacement over ts-node. I linked the development branch in the issue. This is a complex issue, so I am going to do my best to explain the landscape today, the issues with it, and what tsx solves for us.

How cypress works with ts-node today

For end users today, we currently use ts-node to:

  • compile typescript and esm via ts-node/esm with --experimental-specifier-resolution=node flag. We detect this based on the users package.json.
  • typescript via cjs uses the default ts-node loader.
  • cjs/esm without typescript uses node only.

See ProjectConfigIpc.ts for more details.

Issues with ts-node today

  1. The user config process is run by cypress on a subprocess, which is all run in commonjs currently. The interoperability between ESM and CJS inside the node.js runtime is still something that isn't well supported. In other words, the process needs to either be executed as ESM or CJS. It cannot handle both. For this reason, commonjs is forced as the module for typescript users. This is explained fairly well in our docs.
    Unfortunately, this leads to a whole new superset of problems when can be seen in this PR as well as a few of the issues linked in this issue.
  2. Cypress also only checks the tsconfig.json in the users project root, even though cypress recommends having it's own tsconfig.json. Given this is mostly to add cypress type checking to your editor in the cypress directory, it has two pitfalls:
    • Likely will not apply type checking to component tests, which commonly live outside the cypress directory.
    • tsconfig.json is not used to transpile the cypress config, which may be a problem for users who have complex plugin configurations.
      Ideally, even if users don't have a tsconfig, cypress in 99% of cases for simple configs should be able to parse the file.
  3. ts-node has not had any active development in over 2 years and does not appearing to be actively maintained

Other Options Explored

  • swc: I briefly explored swc a few months back, but did not have much luck in getting it to work seamlessly with user config like tsx.

Why tsx?

Experimenting with tsx, it looks like we can fix most of the above issues with ts-node.

  • tsx works as a generic node loader with typescript support, meaning it can handle any esm, cjs, and typescript variant. This allows us to use the same loader for any config format, eliminating the logic needed to try and discover the expected config module format.
  • It also has seamless cjs and esm import support, meaning we can run a user's cypress config with an independent tsconfig.json and it be compatible with our run time. This completely gets rid of the need to override a users typescript config, eliminating many of the bugs associated with such behavior.
  • We can also feed tsx a unique tsconfig.json path, allowing us to actually use the tsconfig.json defined in the cypress directory or whatever directory their cypress tests live.

Proof of Concept

I spiked into this in the chore/experiment_with_tsx, where we can see fully passing tests in dc10aaf via this circle workflow.

However, you can see the binary build is failing with:

Error [ERR_INTERNAL_ASSERTION]: Code: ERR_MODULE_NOT_FOUND; The provided arguments length (2) does not match the required ones (3).
This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
Please open an issue with this stack trace at https://github.com/nodejs/node/issues

This is most likely an issue with the data-context package, which doesn't compile itself and is instead references as raw typescript inside @packages/server, @packages/graphql to manage state while cypress is running, and is also used in @packages/app, @packages/launchpad, and @packages/frontend-shared for cy-in-cy tests to mutate server state for us to be able to test cy-in-cy correctly. We HAVE seen this issue inside our system tests, and have to offer a test specific tsconfig.json to work around the issue (which can be seen here).

Since the tsconfig.jsons per package do NOT compile the data-context package directly, the package is left configless in tsx, in which tsx then tries to compile the file, which fails because we are using experimentalDecorators in the data-context package. experimentalDecorators uses the ECMA stage 2 proposal of the decorators (which was introduced pre typescript 5), which takes 3 arguments. TypeScript 5 added support for the TC39 Stage 3 decorators, which only takes 2 arguments. I believe this is why we are seeing this issue.

We would likely get better output when we update Electron 35 as it is on Node 22 (see issue).

To make this simpler, we likely should migrate the data-context package away from the experimentalDecorators, which I believe is only used by the cached decorator.

We also might want to consider removing experimentalDecorators for the reporter used by mobx, but this should not be of concern here because that package is bundled into the browser and runs outside of the node context.

Additionally with this work, we want to drop the @packages/ts package and hoist the tsconfig.json from that package into the root directory of cypress. This would mean we need to drop references of require('@packages/ts/register') inside packages main property and replace it with require('tsx/cjs'). We would also need to get rid of tsNode.register functionality that is provided by the package, which DOES happen during the smoke test / non snapshotted binary currently

references

Issues with Sinon and TSX

One of the larger pain points for us is trying to use tsx with sinon. This is largely due to swc and tsx/esbuild following the esm specification and not allowing exports to be overwritten (see esbuild issue and sinon/swc issue), which makes the near impossible to stub with sinon. This is not an issue with modern testing frameworks like jest or vitest because these frameworks intercept the loading of the actual module and replace it with a mock, vs sinon which tries to mock the module itself (explained well by these comments 1 & 2).

This is not an issue with ts-node since ts-node just uses the tsc compiler to transpile down to commonjs.

This gives us a few options to work around this issue with mocha until we are able to migration:

  • patch tsx's invocation of esbuild to do a workaround similar to this, which is for sure a code smell.
  • continue to use ts-node and mocha/sinon in our unit tests on a per package basis until we can migrate to vitest.
  • try to rewrite some of the source code to be more stub-able (which can be seen in some of the npm packages on this branch).

Personally, I think option 2 makes the most sense as resources are better spent on moving to something that fits the current landscape better vs trying to monkey-patch around the problem.

Other considerations

Likely with Cypress 15, we want to drop support for TypeScript 4 to make the permutations of tsconfigs we feed to tsx smaller. tsx doesn't explicitly state that TypeScript 4 is not supported, but it is likely a good idea to limit our footprint of things that could possibly go wrong.

@AtofStryker
Copy link
Contributor

I also took a look into jiti today. There doesn't look to be a way to provide a tsconfig. My guess is that is because it is using babel?

I am trying to get a basic node registration of an import loader with jiti but its failing with an ES module interop issue. It looks like it cannot take .js extensions that are commonjs by default and requires the .cjs extension.

I will say the documentation is a lot more clear cut with tsx.

Some additional resources to consider:

@AtofStryker
Copy link
Contributor

We also use the tsconfig.json in the cypress directory (if one exists) today in the preprocessor. If one doesn't exist, we apply the root. This could be an issue for CT files since they usually live in the src/ directory outside of cypress.

For the cypress.config.ts, we need to consider which tsconfig should get applied. If cypress/tsconfig.json is for the browser, and cypress.config.ts runs in the node process, it makes sense to me for that config to have it's own tsconfig. In most cases it likely doesn't matter, but we likely want a way for users to pass in their own tsconfig to transpile their cypress.config.ts.

@AtofStryker
Copy link
Contributor

closing as the spike should be complete

@AtofStryker
Copy link
Contributor

another note: whatever solution we wind up going with we need to testthe migration manager from v9 -> v14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore Work is required w/ no deliverable to end user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants