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

Apps TS config not respected when running E2E suite #6917

Closed
adambrgmn opened this issue Sep 2, 2021 · 10 comments · Fixed by #7629
Closed

Apps TS config not respected when running E2E suite #6917

adambrgmn opened this issue Sep 2, 2021 · 10 comments · Fixed by #7629
Labels
outdated scope: testing tools Issues related to Cypress / Jest / Playwright / Vitest support in Nx type: bug

Comments

@adambrgmn
Copy link

First off all – thanks for this great project! It's making our lives so much easier 🙏 But... We've encountered an issue when trying to upgrade all our NX dependencies to version 12.8.0. It's an inconsistency between running the development server directly and running it via the @nrwl/cypress executor.

Current Behavior

I have been able to reproduce the issue in this repo: adambrgmn/nx-error-repro.

We are using the baseUrl option in one of our apps tsconfig ("baseUrl": "src") and it has been working fine the whole time that we've used NX. And when upgrading from v12.5 to v12.8 of all the NX packages it still works fine when running nx run admin:serve and nx run admin:build. But when running nx run admin-e2e:e2e it fails hard:

Module not found: Error: Can't resolve 'utils/add' in '{project_root}/apps/admin/src/app'

It's like when running the dev server via the E2E executor it ignores the baseUrl configured in our application. It's actually like it ignores the tsconfig in that app completely. Because I've tried to move away from the baseUrl and use a path alias instead. But it is completely ignored.

Expected Behavior

I expect the E2E executor to respect the tsconfig living in the application that it is targeting.

And yes, this seems to be a regression, since this worked fine before trying to upgrade to v12.8.0.

Steps to Reproduce

I have been able to reproduce the whole thing in this repo: adambrgmn/nx-error-repro

  1. Install dependencies (I used yarn)
  2. Run yarn nx run admin:serve and it should run without issues
  3. Run yarn nx run admin:build and it should run without issues
  4. Run yarn nx run admin-e2e:e2e and it should fail, complaining that utils/add can't be resolved

Failure Logs

❯ yarn nx run admin-e2e:e2e --production
yarn run v1.22.10
$ {projectPath}/node_modules/.bin/nx run admin-e2e:e2e --production

> nx run admin-e2e:e2e --production

>  NX  Web Development Server is listening at http://localhost:4200/

Hash: 3b7279ad07af1eb4702e
Built at: 09/02/2021 12:48:25 PM
Entrypoint main = runtime.js runtime.js.map vendor.js main.js main.js.map
Entrypoint polyfills = runtime.js runtime.js.map polyfills.js polyfills.js.map
Entrypoint styles = runtime.js runtime.js.map styles.js styles.js.map
chunk {main} main.js, main.js.map (main) 470 KiB ={runtime}= ={vendor}= [initial] [rendered]
chunk {polyfills} polyfills.js, polyfills.js.map (polyfills) 683 KiB ={runtime}= [initial] [rendered]
chunk {runtime} runtime.js, runtime.js.map (runtime) 0 bytes ={main}= ={polyfills}= ={styles}= ={vendor}= [entry] [rendered]
chunk {styles} styles.js, styles.js.map (styles) 429 KiB ={runtime}= [initial] [rendered]
chunk {vendor} vendor.js (vendor) 1010 KiB ={main}= ={runtime}= [initial] [rendered] split chunk (cache group: vendor) (name: vendor)

ERROR in ./app/app.tsx
Module not found: Error: Can't resolve 'utils/add' in '{projectPath}/apps/admin/src/app'
Could not compile application files

———————————————————————————————————————————————

>  NX   ERROR  Running target "admin-e2e:e2e" failed

  Failed tasks:

  - admin-e2e:e2e

  Hint: run the command with --verbose for more details.

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Environment

>  NX  Report complete - copy this into the issue template

  Node : 14.17.0
  OS   : darwin x64
  yarn : 1.22.10

  nx : Not Found
  @nrwl/angular : Not Found
  @nrwl/cli : 12.8.0
  @nrwl/cypress : 12.8.0
  @nrwl/devkit : 12.8.0
  @nrwl/eslint-plugin-nx : 12.8.0
  @nrwl/express : Not Found
  @nrwl/jest : 12.8.0
  @nrwl/linter : 12.8.0
  @nrwl/nest : Not Found
  @nrwl/next : Not Found
  @nrwl/node : Not Found
  @nrwl/nx-cloud : Not Found
  @nrwl/react : 12.8.0
  @nrwl/schematics : Not Found
  @nrwl/tao : 12.8.0
  @nrwl/web : 12.8.0
  @nrwl/workspace : 12.8.0
  @nrwl/storybook : 12.8.0
  @nrwl/gatsby : Not Found
  typescript : 4.3.5
@adambrgmn adambrgmn changed the title TS baseURL not respected when running E2E suite Apps TS config not respected when running E2E suite Sep 2, 2021
@adambrgmn
Copy link
Author

adambrgmn commented Sep 2, 2021

Okey, a bit more context while I'm trying to decipher what is going wrong here.
This is most likely be a webpack issue, rather than a typescript issue. Since the type-checking works fine.

So I took a look into the nx source and eventually ended up in /packages/workspace/src/utilities/run-webpack.ts:43. And if I inspect the config passed here it differs depending on which script executed it.

More specifically the difference is in the config.resolve.plugins and a plugin called described-resolve (the raw output said described-resolve, but I'm now thinking that it is probably the TsConfigPathsPlugin). The baseUrl and absoluteBaseUrl properties differ:

- "baseUrl": "src",
+ "baseUrl": "../..",
- "absoluteBaseUrl": "{project_root}/apps/admin/src"
+ - "absoluteBaseUrl": "{project_root}"

(the additions (green lines) are from running the E2E executor, the removals from the functioning dev server executor)

@adambrgmn
Copy link
Author

adambrgmn commented Sep 2, 2021

After digging into the TSConfigPathsPlugin I found the culprit: https://github.com/nrwl/nx/blob/master/packages/cypress/src/executors/cypress/cypress.impl.ts#L70. It was added in #6706.

As far as I can tell the env variable TS_NODE_PROJECT seem to override any other variables passed to the tsconfig paths plugin. Which means the apps tsconfig will not be respected. Instead the e2e tsconfig will be used for everything running at the same time as the e2e suite.

@adambrgmn
Copy link
Author

@FrozenPandaz sorry to bring you into this, but just wondering if you know the reasoning behind adding the process.env.TS_NODE_PROJECT line in #6706? What does it do?

@leosvelperez leosvelperez added the scope: testing tools Issues related to Cypress / Jest / Playwright / Vitest support in Nx label Sep 3, 2021
@adambrgmn
Copy link
Author

For anyone encountering this same issue I managed to fix an interim solution by modifying the nx source code and using patch-package.

  1. Remove or comment out line 46 in node_modules/@nrwl/cypress/src/executors/cypress/cypress.impl.js
  2. Run yarn patch-package @nrwl/cypress or npx patch-package @nrwl/cypress to keep the patch between installs

You should get a patch similar to this:

diff --git a/node_modules/@nrwl/cypress/src/executors/cypress/cypress.impl.js b/node_modules/@nrwl/cypress/src/executors/cypress/cypress.impl.js
index e03a1d0..9e7656b 100644
--- a/node_modules/@nrwl/cypress/src/executors/cypress/cypress.impl.js
+++ b/node_modules/@nrwl/cypress/src/executors/cypress/cypress.impl.js
@@ -43,7 +43,7 @@ function normalizeOptions(options, context) {
     if (options.tsConfig) {
         const tsConfigPath = path_1.join(context.root, options.tsConfig);
         options.env.tsConfig = tsConfigPath;
-        process.env.TS_NODE_PROJECT = tsConfigPath;
+        // process.env.TS_NODE_PROJECT = tsConfigPath;
     }
     checkSupportedBrowser(options);
     return options;

@JFGagnon
Copy link

@leosvelperez @FrozenPandaz issue is still present in 12.10. Any idea when this will be fixed? This is a breaking change that's preventing us to update to a newer version of Nx

@jeroen-hoekstra
Copy link

Issue is still present in 13.1.3 version of @nrwl/cypress. Doing the patch work @adambrgmn suggested makes our tests work again.

@wrslatz
Copy link
Contributor

wrslatz commented Nov 5, 2021

I think this issue is related, with the linked comment as a potential workaround #1276 (comment)

Edit:
Seems like the need to use Cypress support for Webpack (not easy, doesn't support paths out of the box) since the Nx Cypress Webpack setup being deprecated

@mathiashellsing
Copy link

mathiashellsing commented Nov 5, 2021

Wow .. debugging all morning and @adambrgmn fix made my tests work again.. but would be nice with a better fix than patching ;)

EDIT - instead of touching node_modules-files; i could also copy my nx tsconfig.base.json compilerOptions.paths to e2e's tsconfig.e2e.json and be able to run cypress tests right now.

AgentEnder added a commit to AgentEnder/nx that referenced this issue Nov 5, 2021
@AgentEnder
Copy link
Member

Hey, I'm putting a PR in to fix our generator but this seems to be caused by the tsconfig option being passed in the cypress configuration.

For example, in the reproduction from @adambrgmn

  "e2e": {
    "executor": "@nrwl/cypress:cypress",
      "options": {
        "cypressConfig": "apps/admin-e2e/cypress.json",
        "devServerTarget": "admin:serve",
-       "tsConfig": "apps/admin-e2e/tsconfig.json"
        },
        "configurations": {
          "production": {
            "devServerTarget": "admin:serve:production"
          }
        }
      },

This should work out of the box, as long as your e2e projects tsconfig is named tsconfig.json. If it isn't, I'd recommend renaming it as such.

@github-actions
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: testing tools Issues related to Cypress / Jest / Playwright / Vitest support in Nx type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants