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

fix(misc): update e2e config generators to align with new TS solution setup #29638

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

jaysoo
Copy link
Member

@jaysoo jaysoo commented Jan 15, 2025

This PR updates the @nx/detox:app generator to match the new TS solution setup. The @nx/cypress:configuration and @nx/cypress:configuration generators are also updated so that they can be run on existing projects and generator the correct tsconfig files.

The Playwright/Cypress example can be seen as follows:

# Skip e2e
nx g @nx/react:app apps/demo --bundler vite --e2eTestRunner none

# now configure e2e
nx g @nx/playwright --project demo

Now if you add this line to apps/demo/e2e/example.spec.ts:

const x: number = 'a';

And run nx typecheck demo, it will pass. This happens because the e2e/**/*.ts pattern is missing. Thus, we need to ensure that a tsconfig.e2e.json project is added for the Playwright spec files. Same thing with Cypress.

The Detox generator does not support adding configuration to existing project, so we don't quite get the same problem. The fix for Detox is just to make sure the tsconfig content is not following the old (integrated) version, but the updated TS solution version.

Current Behavior

Detox TS setup is incorrect. Running Cypress and Playwright configuration generator on existing projects generate invalid setup, such that spec files are not typechecked.

Expected Behavior

E2E generators should all generate correct TS setup.

Related Issue(s)

Fixes #

Copy link

vercel bot commented Jan 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview Jan 16, 2025 4:51pm

Copy link

nx-cloud bot commented Jan 15, 2025

View your CI Pipeline Execution ↗ for commit d55ad0e.

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 33m 48s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 18s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 2s View ↗
nx-cloud record -- nx format:check --base=8a6e4... ✅ Succeeded 29s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 27s View ↗
nx documentation --no-dte ✅ Succeeded 46s View ↗
nx affected -t e2e-macos-local --parallel=1 --b... ✅ Succeeded 21m 37s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-16 17:29:32 UTC

@jaysoo jaysoo force-pushed the fix/e2e-config-generator branch 3 times, most recently from 43b567d to 72f9c8f Compare January 15, 2025 21:14
@@ -1,7 +1,6 @@
import { defineConfig, devices } from '@playwright/test';
import { nxE2EPreset } from '@nx/playwright/preset';
<% if(!webServerCommand || !webServerAddress) { %>// eslint-disable-next-line @typescript-eslint/no-unused-vars <% } %>
import { workspaceRoot } from '@nx/devkit';
<% if (webServerCommand && webServerAddress) { %>import { workspaceRoot } from '@nx/devkit';<% } %>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is unused unless both options are set, which leads to typecheck errors in the new setup. We should just not import it if we're not going to use it.

@jaysoo jaysoo marked this pull request as ready for review January 15, 2025 21:21
@jaysoo jaysoo requested review from a team as code owners January 15, 2025 21:21
@jaysoo jaysoo force-pushed the fix/e2e-config-generator branch from 72f9c8f to 19eb0d0 Compare January 15, 2025 21:22
@jaysoo jaysoo force-pushed the fix/e2e-config-generator branch from 19eb0d0 to 5708c40 Compare January 16, 2025 14:12
@jaysoo jaysoo requested review from a team as code owners January 16, 2025 14:12
@jaysoo jaysoo requested a review from mandarini January 16, 2025 14:12
@jaysoo jaysoo force-pushed the fix/e2e-config-generator branch from 5708c40 to d55ad0e Compare January 16, 2025 16:49
@jaysoo jaysoo merged commit 1e032fb into master Jan 16, 2025
6 checks passed
@jaysoo jaysoo deleted the fix/e2e-config-generator branch January 16, 2025 18:10
FrozenPandaz pushed a commit that referenced this pull request Jan 17, 2025
… setup (#29638)

This PR updates the `@nx/detox:app` generator to match the new TS
solution setup. The `@nx/cypress:configuration` and
`@nx/cypress:configuration` generators are also updated so that they can
be run on existing projects and generator the correct tsconfig files.

The Playwright/Cypress example can be seen as follows:

```shell
# Skip e2e
nx g @nx/react:app apps/demo --bundler vite --e2eTestRunner none

# now configure e2e
nx g @nx/playwright --project demo
```

Now if you add this line to `apps/demo/e2e/example.spec.ts`:
```
const x: number = 'a';
```

And run `nx typecheck demo`, it will pass. This happens because the
`e2e/**/*.ts` pattern is missing. Thus, we need to ensure that a
`tsconfig.e2e.json` project is added for the Playwright spec files. Same
thing with Cypress.

The Detox generator does not support adding configuration to existing
project, so we don't quite get the same problem. The fix for Detox is
just to make sure the tsconfig content is not following the old
(integrated) version, but the updated TS solution version.

## Current Behavior
Detox TS setup is incorrect. Running Cypress and Playwright
configuration generator on existing projects generate invalid setup,
such that spec files are not typechecked.

## Expected Behavior
E2E generators should all generate correct TS setup.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #

(cherry picked from commit 1e032fb)
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants