Unable to compile TypeScript with "moduleResolution": "bundler" option in v5
#26308
Comments
astone123
changed the title
"moduleResolution": "bundler" causes errors"moduleResolution": "bundler" option in v5
|
@MasonM thanks for including the reproduction, routing this to our e2e team |
|
This seems to fix it: develop...MasonM:cypress:fix-26308 But I only tested by manually applying this change to |
|
|
|
@lmiller1990 Probably because it's getting overridden to |
|
@lmiller1990 That's odd. The reproduction repository doesn't have |
|
@MasonM I removed my post, I was on the wrong branch. The reproduction does indeed exhibit the issue. |
|
Let's open a PR and see if the test suite passes! |
|
@lmiller1990 Okay, done: #26415 Seems the tests aren't running, probably because I didn't sign the CLA. But I can't do that without permission from my employer, and that process generally takes several months, so would you or @MattyBalaam mind entering the PR instead? |
|
Actually, this is a little confusing - the issue is that What is the goal here, exactly? Just to clarify -- If it's just to make it work, this will be just fine. The concern I have is looking at the Is this correct? Or do you want to enforce I think your fix is probably okay, since the goal is just to have things work as you'd generally expect, and I'd say most people expect |
|
@MasonM right, I see - once we clarify the goal here, I can force commit with and override your contribution (and CLA requirement). I can still tag you in the commit message to give some attribution. |
|
In my project, we don't have any tests written in TypeScript, only application code. In other words, the mere presence of a |
|
I wrote the original reproduction because in our codebase we do use TypeScript in a monorepo and we explicitly want to use the The horrible workaround we have at the moment is to put a tsconfig with |
|
If the goal is what @MattyBalaam wrote, then I am not sure the proposed fix of hard-coding @MattyBalaam to help drive this forward, can you offer insight into why you are choosing to use |
|
Hi, I'm on holiday at the moment so can't do a big deep dive, but the moduleResolution is set in This test case is maybe a little inaccurate for our use case which is component tests, and I've just noticed the config here is for e2e (I forked off someone else's example) We are building out apps using remix which uses esbuild, however out component tests are using a webpack config. The bundler config allows us to export a stricter set of files from our shared UI packages using exports rather than main in our package.json. |
|
Sure no problem - no rush on this. I think there's a bit of confusion here - if the project has A lot of projects I've seen have started doing I think we should come up with a better general solution so this entire process is less confusing. When you've got some bandwidth, we can look to explore some solutions. I wonder if we can adopt esbuild too - it's fast, and a lot more simple to use. |
|
We're also experiencing this problem. Is there a recommended workaround until "moduleResolution": "bundler" is supported? |
|
Oh you know what, I think I do have a good solution. Let me clarify, to make sure it's clear what's going on. Most users want the The side effect is Cypress uses Assume this use case, what you can do is specify a https://github.com/MattyBalaam/cypress-ts-import/pull/1/files For completeness, the code: {
+ "ts-node": {
+ "compilerOptions": {
+ "module": "es2015",
+ "moduleResolution": "node"
+ }
+ },
"compilerOptions": {
"module": "es2015",
"moduleResolution": "bundler"
}
}cc @filiptammergard @MattyBalaam @MasonM - this should help! I think this is more correct, actually. You have complete control over how both TS pipelines (your Node.js for https://github.com/cypress-io/cypress/pull/26415/files is still probably relevant and makes sense, we can get this PR ready, too. The only non-solved use case is actually using |
|
@lmiller1990 Looks like a good workaround! While it might be more correct, it also adds extra config to maintain and it's not obvious it's related to I don't see why someone would want to use I'd love to see #26415 happening to make it possible to keep the project's But good workaround in the meantime! |
|
#26415 probably still makes sense, main concern is it might break some existing workflows. I'll look into it and see if it can merge as-is, or if it'll need to wait for a major. |
I just took a look at our code again and noticed since I created the original test case here we had also added a |
nagash77
added
Triaged
|
Any news on that? |
|
I don't think anyone is actively working on this. The proper solution doesn't seem clear, either (any recommendation)? Does #26308 (comment) work for you? |
|
@philipp-serfling The PR is stalled because I can't sign the CLA, so the Cypress team can't accept it. If you can sign the CLA, then feel free to open an identical PR. |
|
@MasonM Oh gosh what a pitty. |
|
I managed to resolve it by running |
|
I think Next.js is broken now too #27448 I will look at picking this one up. |
|
Thanks @lmiller1990 ! For background, it was changed in vercel/next.js#51957, so broken since a few versions now. Also, just generally, would be great to get Cypress a bit more stable with TypeScript and ESM, there's a few longer-running bugs which have no good solution to them :) Can feel pretty broken for new users, and we have been suggesting more users switch to Playwright now because of this. Maybe the leadership would agree with you spending a few days / weeks fixing everything. |
WorkaroundFor people looking for a temporary workaround, the workaround posted above by @lmiller1990 worked for us too: Adding the following (old, obsolete)
{
"compilerOptions": {
"module": "ESNext",
"moduleResolution": "Bundler"
},
+ // Old "moduleResolution": "Node" option required for Cypress
+ // https://github.com/cypress-io/cypress/issues/26308#issuecomment-1663592648
+ //
+ // TODO: Remove when issue is resolved https://github.com/cypress-io/cypress/issues/27448
+ "ts-node": {
+ "compilerOptions": {
+ "module": "ESNext",
+ "moduleResolution": "Node"
+ }
+ }
} |
This works for me |
|
I too was having the same issue, so decided to try with a freshly created Vite app. To my surprise, I was not able to recreate the issue with the fresh Vite app, although it uses This is how I created the Vite app: npm create vite@latest # choose options react and typescript
npm install cypress --save-dev
npx cypress open |
Also using this workaround |
|
Working on this in #27484 |
|
@lmiller1990 Is this available to use? If so, what version? |
|
@filiptammergard from the changelog changes in #27484 it seems that 12.17.5 will include this change, to be released on 29 August 2023: |
|
Released in Cypress 13.0.0. |
|
Reopened due to the issue being re-reported and the fix not fully resolving the issue. Please see this comment for workaround suggestions: #27731 (comment). |
|
Going to actually close this as a duplicate of #27731. Please follow that issue if you're encountering this error. |
Current behavior
If you have a TypeScript project using the new
"moduleResolution": "bundler"setting introduced in TypeScript 5, any attempts to run tests will cause the following error:Desired behavior
Tests run successfully
Test code to reproduce
Courtesy of @MattyBalaam: https://github.com/MattyBalaam/cypress-ts-import
Cypress Version
12.8.1
Node version
18.11.0
Operating System
macOS 12.6
Debug Logs
Expand
Other
This was originally reported in #26148, but was determined to be a separate issue. From looking at the code, I believe the following line is causing this behavior:
cypress/packages/server/lib/plugins/child/ts_node.js
Line 25 in e6b2466
There is a workaround, but it only works if you don't have any tests written in TypeScript: set the environment variable
CYPRESS_INTERNAL_NO_TYPESCRIPT=1to disable TypeScript entirely:cypress/packages/server/lib/util/resolve.js
Lines 12 to 14 in 74ada11
The text was updated successfully, but these errors were encountered: