Skip to content

chore: upgrade nx #3774

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

Merged
merged 73 commits into from
May 15, 2025
Merged

chore: upgrade nx #3774

merged 73 commits into from
May 15, 2025

Conversation

ScriptedAlchemy
Copy link
Member

Description

This pull request includes updates across multiple files to improve Cypress configuration, enhance ESLint ignore patterns, upgrade dependencies, and address module federation imports. Below is a summary of the most important changes:

Cypress Configuration Updates

  • Updated the e2e field in Cypress configuration files to include injectDocumentDomain: true and added a comment to encourage the use of cy.origin() for navigating between domains. This change was applied to multiple applications, including 3000-home, 3001-shop, 3002-checkout, bundle-size, manifest-demo/webpack-host, modernjs-ssr/host, modernjs, router-demo/router-host-2000, and runtime-demo/3005-runtime-host. [1] [2] [3] [4] [5]

ESLint Ignore Pattern Enhancements

  • Updated .eslintrc.json files across various packages and applications to include additional ignore patterns for files matching vite.config.*.timestamp* and vitest.config.*.timestamp*. This change was made in files such as website/.eslintrc.json, packages/esbuild/.eslintrc.json, packages/modernjs/.eslintrc.json, and others. [1] [2] [3] [4] [5]

Dependency Upgrades

  • Upgraded several dependencies in package.json, including Cypress (13.15.014.3.3), TypeScript (5.5.25.7.3), and various @nx packages (20.1.421.0.3). Other notable upgrades include express, vite, webpack, and Storybook-related packages. [1] [2] [3] [4]

Module Federation Import Adjustment

  • Updated the import path for withModuleFederation in reactStorybook/webpack.config.prod.js to reflect its new location under @nx/module-federation/webpack.

Build Configuration Improvements

  • Added a cache property to the dependsOn field for certain tasks in nx.json to improve build performance. Additionally, removed the useLegacyCache property. [1] [2]

Related Issue

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the documentation.

Copy link

netlify bot commented May 14, 2025

Deploy Preview for module-federation-docs ready!

Name Link
🔨 Latest commit 9e6e0dc
🔍 Latest deploy log https://app.netlify.com/projects/module-federation-docs/deploys/68265ca079f9870008839acb
😎 Deploy Preview https://deploy-preview-3774--module-federation-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

changeset-bot bot commented May 14, 2025

🦋 Changeset detected

Latest commit: 9e6e0dc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@module-federation/storybook-addon Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ScriptedAlchemy ScriptedAlchemy requested a review from Copilot May 15, 2025 22:41
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request upgrades dependencies and updates configuration files across several applications and workflows to improve end-to-end testing, build performance, and module federation imports. Key changes include:

  • Enhancing Cypress configurations with the injectDocumentDomain option and additional comments on domain navigation.
  • Upgrading dependencies such as Cypress, TypeScript, and various @nx packages while adjusting related build/test commands.
  • Updating GitHub workflows by adding timeout settings and refining commands for killing processes and running tests.

Reviewed Changes

Copilot reviewed 57 out of 57 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
apps/3002-checkout/project.json Updated e2e and test commands with improved port termination and parallelism.
apps/3002-checkout/cypress.config.ts Applied updated Cypress preset with injectDocumentDomain option and comments.
apps/3001-shop/project.json Refined production configuration with new kill-port and wait-on commands.
apps/3001-shop/cypress.config.ts Updated Cypress configuration to include injectDocumentDomain and guidance.
apps/3000-home/project.json Adjusted development and production commands for synchronized testing.
apps/3000-home/cypress.config.ts Updated configuration similar to other apps with injectDocumentDomain.
.vscode/settings.json Added new nxConsole.generateAiAgentRules configuration.
Various .github/workflows/*.yml Introduced timeout-minutes and refined commands for multi-app E2E testing.
.cursorignore and .cursor/mcp.json Updated ignore patterns and added MCP server configuration.
.changeset/spotty-buttons-cough.md Recorded changeset for a fix relating to type errors with the nx update.
Comments suppressed due to low confidence (1)

.cursorignore:27

  • [nitpick] Removing 'nx.json' from the ignore list may result in tracking generated configuration changes; please ensure this is intentional to avoid unintended repository clutter.
   -nx.json

"forwardAllArgs": false
},
{
"command": "echo 'done'",
Copy link
Preview

Copilot AI May 15, 2025

Choose a reason for hiding this comment

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

[nitpick] The use of 'echo "done"' as the final step in the production command sequence seems to act as a placeholder rather than a robust verification step; consider replacing it with an explicit E2E test confirmation or removal if not needed.

Suggested change
"command": "echo 'done'",
"command": "nx run 3001-shop:e2e:production",

Copilot uses AI. Check for mistakes.

Comment on lines +49 to +61
killall node
npx nx run 3000-home:test:e2e:production

- name: E2E Test for Next.js Prod - Shop
if: steps.check-ci.outcome == 'success'
run: |
killall node
npx nx run 3001-shop:test:e2e:production

- name: E2E Test for Next.js Prod - Checkout
if: steps.check-ci.outcome == 'success'
run: |
killall node
Copy link
Preview

Copilot AI May 15, 2025

Choose a reason for hiding this comment

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

[nitpick] Using 'killall node' may unintentionally terminate other Node processes running in the environment; consider using a more targeted approach (e.g., a specific port or process identifier) to stop only the intended processes.

Suggested change
killall node
npx nx run 3000-home:test:e2e:production
- name: E2E Test for Next.js Prod - Shop
if: steps.check-ci.outcome == 'success'
run: |
killall node
npx nx run 3001-shop:test:e2e:production
- name: E2E Test for Next.js Prod - Checkout
if: steps.check-ci.outcome == 'success'
run: |
killall node
PID=$(lsof -t -i:3000) && kill -9 $PID || echo "No process running on port 3000"
npx nx run 3000-home:test:e2e:production
- name: E2E Test for Next.js Prod - Shop
if: steps.check-ci.outcome == 'success'
run: |
PID=$(lsof -t -i:3001) && kill -9 $PID || echo "No process running on port 3001"
npx nx run 3001-shop:test:e2e:production
- name: E2E Test for Next.js Prod - Checkout
if: steps.check-ci.outcome == 'success'
run: |
PID=$(lsof -t -i:3002) && kill -9 $PID || echo "No process running on port 3002"

Copilot uses AI. Check for mistakes.

@@ -84,14 +88,37 @@
"parallel": true,
"commands": [
{
"command": "lsof -i :3000 || nx run 3000-home:serve",
"command": "npx kill-port 3000 3001 3002",
Copy link
Preview

Copilot AI May 15, 2025

Choose a reason for hiding this comment

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

[nitpick] There is a mix of using 'npx kill-port' in some files and 'killall node' in others; consider standardizing the approach across the codebase for clarity and maintainability.

Copilot uses AI. Check for mistakes.

@ScriptedAlchemy ScriptedAlchemy merged commit 2ca17f8 into main May 15, 2025
24 of 26 checks passed
@ScriptedAlchemy ScriptedAlchemy deleted the update-nx branch May 15, 2025 22:44
mitchellrj pushed a commit to mitchellrj/module-federation-core that referenced this pull request May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant