Skip to content

fix(dashboard): resolve Next 16 Webpack node:fs imports and login page unmounted state warning - #13509

Closed
joesalaz wants to merge 2 commits into
diegosouzapw:release/v3.8.51from
joesalaz:fix/next16-node24-webpack-fixes
Closed

joesalaz wants to merge 2 commits into
diegosouzapw:release/v3.8.51from
joesalaz:fix/next16-node24-webpack-fixes

Conversation

@joesalaz

Copy link
Copy Markdown

Fix(dashboard): resolve Next 16 Webpack node:fs imports and login page unmounted state warning

Summary

  • Resolves Next.js 16 Webpack UnhandledSchemeError: Reading from "node:fs" when client components (ModelSelectModal.tsx) transitively import provider model constants.
  • Adds an isMounted guard to src/app/login/page.tsx to prevent unmounted state updates during Hot Module Replacement (HMR).
  • Fixes better-sqlite3 native dependency installation and allowScripts entries for Node 24 / npm 11.

Related Issues

  • Related to Next 16 / Node 24 dev server compatibility.

Validation

  • Change type: UI / build-deploy
  • Focused tests and category gates from the golden path (npm run lint, npm run typecheck:core)
  • npm run lint
  • Reconciled with the current active release base (release/v3.8.51); focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • src/app/login/page.tsx (updated with isMounted lifecycle guards)
  • open-sse/utils/cursorAgentCliVersion.ts (wrapped Node fs/os/path imports for browser safety)

Coverage Notes

  • Changes touch src/app/login/page.tsx, open-sse/utils/cursorAgentCliVersion.ts, and next.config.mjs. All core types and lint gates pass cleanly (npm run lint and npm run typecheck:core both return 0 errors).

Reviewer Notes

  • Safe, non-breaking fix for Next.js 16 Webpack bundling under Node 24 / npm 11 development environments.

Copilot AI lite review requested due to automatic review settings September 13, 2026 03:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved critical webpack-test and ESM-loading findings, plus an incomplete login unmount guard, block approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR addresses Next 16/Node 24 dashboard compatibility, login lifecycle handling, browser-safe Node imports, dependency installation, and release documentation.

Changes:

  • Updates login imports and mounted-state guards.
  • Adjusts Cursor CLI filesystem handling and Next/Webpack configuration.
  • Updates native dependency approvals, lockfile metadata, and changelog.
File summaries
File Review summary
src/app/login/page.tsx Moderate (3 votes): Recheck mount state after await res.json(). Nit (3 votes): Add regression coverage for unmount behavior.
package.json Updates dependency and install-script allowlists.
package-lock.json Updates dependency lockfile metadata.
open-sse/utils/cursorAgentCliVersion.ts Critical (3 votes): Bare require is incompatible with ESM and breaks filesystem detection/cache reads.
next.config.mjs Critical (2 votes): The new replacement plugin conflicts with the existing webpack-config test fixture and assertions.
changelog.d/fixes/13300-next16-node24-webpack-hmr-fixes.md Adds the release note for the compatibility fixes.
Review details

Suppressed comments (1)

src/app/login/page.tsx:105

  • The new lifecycle guard is scoped only to checkAuth; the submit request still calls setError in the catch and setLoading(false) in finally after the page may have been unmounted by HMR or navigation. That leaves the same unmounted-update path in handleLogin. Share a component-level mounted/abort guard with this handler and guard its setters as well.
    } catch (_err) {
      setError(t("errorOccurredRetry"));
    } finally {
      setLoading(false);
    }
  • Files reviewed: 5/6 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread next.config.mjs Outdated
Comment on lines +410 to +414
config.plugins.push(
new webpack.NormalModuleReplacementPlugin(/^node:(.*)$/, (resource) => {
resource.request = resource.request.replace(/^node:/, "");
})
);
Comment thread open-sse/utils/cursorAgentCliVersion.ts Outdated
Comment on lines +18 to +22
nodeFs = require("fs");
// eslint-disable-next-line @typescript-eslint/no-require-imports
nodeOs = require("os");
// eslint-disable-next-line @typescript-eslint/no-require-imports
nodePath = require("path");
Comment thread src/app/login/page.tsx
if (!isMounted) return;

if (res.ok) {
const data = await res.json();
Comment thread src/app/login/page.tsx
Comment on lines +25 to +27
const timer = setTimeout(() => setMounted(true), 0);
return () => clearTimeout(timer);
}, []);
…mount

- Guard the node:-prefix NormalModuleReplacementPlugin against test-fixture webpack mocks in next.config.mjs
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks for digging into this — the underlying node:fs bug you found is real, but there's
already a much smaller, more targeted fix for it in #13604 (extracts just the pin constant,
no webpack config changes needed), so we'll go with that one for the bundling issue.
Your login-page unmounted-state fix looks independently useful, though — would you mind
splitting that into its own PR? It's unrelated to the build issue and easier to review (and
land) on its own. Same for the better-sqlite3/trustedDependencies package.json changes — those
look like they belong in a separate, dedicated PR with their own rationale.

Triage note: this is the review recommendation — the close itself happens only after the maintainer's per-PR sign-off (and, where a superseding PR is named, after it has landed). Nothing is being closed by this comment.

@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks for the fix! The node:fs client-bundle break landed via #13436, which moves the same server-only imports out of the client chunk graph and also widens the client-bundle guard so any Node builtin reaching a client entry fails the test (with resolved-edge caching so the guard stays fast). With that merged this PR no longer applies cleanly and its change is covered. Closing as covered by #13436.

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.

3 participants