fix: force moduleResolution: 'node' when ts-node is registered in the… #28709
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… cypress process to make sure value is compatible with commonjs (which is already forced). [run ci]
AtofStryker
force-pushed
the
fix/set_module_resolution_with_commonjs
branch
from
7d295fa
to
c6e0050
Compare
cli/CHANGELOG.md
Outdated
| @@ -4,7 +4,7 @@ | |||
| _Released 1/16/2024 (PENDING)_ | |||
|
|
|||
| **Bugfixes:** | |||
|
|
|||
| - Force `moduleResolution` to `node` when `typescript` projects are detected to correctly run Cypress. This change should not have a large impact as `commonjs` is already forced when `ts-node` is registered. Fixes [#27731](https://github.com/cypress-io/cypress/issues/27731). | |||
There was a problem hiding this comment.
maybe worth noting it doesn't work with the esm loader
There was a problem hiding this comment.
or maybe that it doesn't impact it?
jordanpowell88
approved these changes
Jan 16, 2024
AtofStryker
commented
Jan 16, 2024
mschile
approved these changes
Jan 16, 2024
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
… cypress process to make sure value is compatible with commonjs (which is already forced). [run ci]
Additional details
This PR attempts to fix #27731, which was reopened when #26308 did not resolve the issue.
Currently in Cypress, if a user has
typescriptinstalled and the project is NOT an ES Module (viatype: 'module'), Cypress sets the module tocommonjswhen registeringts-nodeto load the project's config. This was to guarantee that cypress could run the transpiled config in its owned node process as node can natively runcommonjswith ease. This however started being a problem when newmoduleResolutionproperties were introduced to newer versions of typescript that are incompatible withcommonjs.To avoid this issue, this PR sets the
moduleResolutiontonodeto guaranteecommonjsas a module configuration will work. #26308 attempted to fix this in a similar way, except themoduleResolutionsetting tonodehappened behind toTS_NODE_COMPILERflag, which is only set when a user provides a custom compiler, which the Cypress monorepo does here, which made the issue appear to be fixed. This isn't usually set in production, hence why #27731 was opened.This should resolve the issues with
next.jsscaffolding, but e need to figure out a more long term solution to processing the users cypress config with typescript. Right now we use the user's typescript configuration to transpile files executed by the cypress node child process, which is a gamble on compatibility (hence the forcing, but isn't really an option withts-node/esmdue to TypeStrong/ts-node#1997 (comment)).To verify the issue, standalone binaries have been build on the commit in this PR 0481c7a. From testing, it looks like the fix works.
Steps to test
Since reproducing this issue is difficult in the cypress monorepo due to the
TS_NODE_COMPILERflag set, standalone binaries have been build on the commit in this PR 0481c7a. Follow thenext.jsreproduction steps in #27731 to create a newnext.jsproject with the CLI, install cypress, and make sure a user can set up their cypress project for the first time without error and run tests.How has the user experience changed?
next.js@14with cypress e2e tests and typescript installed should run out of the box. This PR does NOT resolve CT support fornext.js@14, mentioned in #28185.PR Tasks
cypress-documentation? (N/A)type definitions? (N/A)