Skip to content

Fix Content route commits on slow networks#2372

Open
3mdistal wants to merge 2 commits into
mainfrom
codex/content-slow-network-route-commit
Open

Fix Content route commits on slow networks#2372
3mdistal wants to merge 2 commits into
mainfrom
codex/content-slow-network-route-commit

Conversation

@3mdistal

Copy link
Copy Markdown
Contributor

Summary

  • let ordinary Content path navigation commit without waiting for the root locale loader, while preserving React Router's action-driven revalidation
  • keep the pending Page choice usable while collaboration connects; authorization, body hydration, and database conversion still gate it
  • add focused route and pending-choice coverage plus a Content changelog entry

This is the intentionally separate follow-up to the slow-network issue found while validating #2363. It does not modify or absorb that PR.

Verification

  • pnpm exec vitest --run app/root.revalidation.test.ts app/components/layout/Layout.layout.test.ts app/components/editor/DocumentEditor.layout.test.ts app/components/editor/body-hydration.test.ts app/components/sidebar/DocumentSidebar.layout.test.ts — 65 passed
  • pnpm typecheck — passed
  • pnpm build — passed
  • pnpm exec oxfmt ... on all modified TypeScript files — clean
  • merge-tree checks against current origin/main and the Fix new-page database conversion race #2363 QA head — clean

Real Content UI QA

Ran the frozen story in a disposable combined checkout of this branch plus #2363, without stacking either PR:

  • under an 8-second global CDP network delay, click → /page/<id> history.pushState measured 65.3 ms; the pending chooser rendered with Page enabled and Database disabled
  • keyboard activation of disabled Database was rejected and produced zero create-content-database requests
  • after persistence, one Database click produced exactly one conversion request, kept the route stable, and completed without reload
  • back/forward preserved the route and Content shell; normal-speed route commit measured 122.7 ms; reload restored the signed-in shell and en-US/ltr locale state
  • no Builder operations or Notion OAuth/sync/write operations were initiated; only the app's passive Notion status reads were observed

Unrelated observations

  • the successful build continues to emit existing repository doctor warnings; none originate from the files changed here

@netlify

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Visual recap — generation failed

The visual recap could not be generated for this pull request. This is informational only and does not block the PR.

Diagnostic:

No plan URL: Repair changed too much of targeted file plan.mdx; expected a localized parser fix.

Agent output: Repaired recap-source.json: both wireframe HTML attributes now use parser-safe double-quoted JavaScript expressions. JSON and non-empty MDX were verified; nothing was published. --- ⠀ 🟢 Visual recap source repaired and ready for the hosted parser retry. stderr: Reading additional input from stdin...

@builder-io-integration builder-io-integration Bot 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.

Builder reviewed your changes — looks good ✅

Review Details

Code Review Summary

PR #2372 makes a focused Content-route change: ordinary navigations no longer wait for the root locale loader, while action/form-driven revalidation continues to follow React Router's default behavior. It also separates the new-document Page chooser from the broader collaboration readiness gate, preserving usability while collaboration connects while still blocking on edit authorization, body hydration, and database conversion; the Database path retains its stricter editor gate.

The approach is sound and well covered by targeted tests. The shouldRevalidate tests verify both ordinary navigation and action submission behavior, and the body-hydration tests cover the intended Page-choice states. The changelog entry is appropriately scoped. One review agent raised a collaboration-loading concern, but source and tests confirm that excluding that state from the Page button is intentional per the PR's stated behavior; the Database button remains protected by the existing editorCanEdit gate.

Risk assessment: Standard, because this changes route revalidation and editor interaction state, but does not alter authentication, payments, schema, or destructive data behavior. No confirmed reportable bugs found.

✅ Focused route and chooser coverage is included.
✅ Action-driven revalidation behavior is preserved.
✅ Database conversion remains protected against interaction while pending.

🧪 Browser testing: Skipped — dev server is healthy, but browser verification was attempted and could not run because Chrome automation tools were unavailable in executor sessions; all planned cases were marked couldnt_verify/env_issue.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

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