Skip to content

test: add failing test with createPortal#1971

Closed
dai-shi wants to merge 5 commits intomainfrom
test/issue-1937-2
Closed

test: add failing test with createPortal#1971
dai-shi wants to merge 5 commits intomainfrom
test/issue-1937-2

Conversation

@dai-shi
Copy link
Member

@dai-shi dai-shi commented Feb 27, 2026

for #1937

@vercel
Copy link

vercel bot commented Feb 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
waku Ignored Ignored Preview Feb 27, 2026 11:14am

Request Review

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 27, 2026

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 27, 2026

Open in StackBlitz

npm i https://pkg.pr.new/waku@1971

commit: d12af9e

@dai-shi dai-shi requested a review from Copilot February 27, 2026 10:52
Copy link

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 PR adds a failing test case to reproduce issue #1937 related to createPortal behavior during popstate navigation. The test verifies that rapid back/forward navigation with portal-rendered content doesn't corrupt the browser history or produce runtime errors.

Changes:

  • Adds new test case that simulates back/forward/back navigation churn with delayed portal route requests
  • Adds a new link in the start page to navigate to a portal-enabled route
  • Updates the RouteState component to conditionally render a portal marker based on query parameter

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
e2e/router-client.spec.ts Adds test case for portal route with popstate navigation that verifies no runtime errors occur and query history is preserved
e2e/fixtures/router-client/src/pages/start.tsx Adds link to navigate to portal-enabled route (/next?portal=1)
e2e/fixtures/router-client/src/components/route-state.tsx Implements conditional portal rendering based on 'portal' query parameter

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +11 to +15
{portal &&
createPortal(
<div data-testid="portal-marker">portal</div>,
document.body,
)}
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The portal is created on every render without checking if document.body is available. In SSR contexts or during hydration, document.body may not be defined, which will cause a runtime error. Add a guard to ensure this only runs on the client side, such as checking if typeof document !== 'undefined' before calling createPortal.

Copilot uses AI. Check for mistakes.
@dai-shi
Copy link
Member Author

dai-shi commented Feb 27, 2026

I don't think it's a reproduction.

@dai-shi dai-shi closed this Feb 27, 2026
@dai-shi dai-shi deleted the test/issue-1937-2 branch February 27, 2026 12:25
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.

2 participants