Skip to content

chore: Allow Portal's hidden SSR placeholder in the SSR test - #4908

Open
TrevorBurnham wants to merge 2 commits into
cloudscape-design:mainfrom
TrevorBurnham:fix-ssr-test-portal-placeholder
Open

chore: Allow Portal's hidden SSR placeholder in the SSR test#4908
TrevorBurnham wants to merge 2 commits into
cloudscape-design:mainfrom
TrevorBurnham:fix-ssr-test-portal-placeholder

Conversation

@TrevorBurnham

@TrevorBurnham TrevorBurnham commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

This PR updates a test assertion that will fail once cloudscape-design/component-toolkit#249 is merged. It should be merged first to prevent the tests in this repo from failing.

Description

Portal renders a hidden probe span on its first render so its layout effect can read ref.current.ownerDocument. That span was gated on typeof document !== 'undefined', so the server emitted nothing while the client's first render emitted the span — a server/client branch on a DOM global, which React reports as Hydration failed because the server rendered HTML didn't match the client before discarding and re-rendering the subtree. The toolkit fix renders the span during SSR too, so both passes agree.

Portal is the sole root of both InternalModal (src/modal/internal.tsx:82) and Tooltip (src/internal/components/tooltip/index.tsx:73), so their server markup is the Portal's output. That takes it from "" to <span style="display:none"></span>, and this assertion fails:

if (componentName === 'modal' || componentName === 'tooltip') {
  // modal and tooltip use portal API which does not work on server and returns an empty content
  expect(content.length).toEqual(0);
}

The comment also encodes the assumption the toolkit fix invalidates — the portal API not working on the server is exactly why the first render has to match, not a reason to emit nothing.

The new assertion in this PR asserts what the test actually cares about: the portaled content is not in the server markup, and whatever is emitted is either nothing or the hidden placeholder.

The SSR test asserts that modal and tooltip emit no server markup at all,
because Portal returns null on the server. component-toolkit#249 makes Portal
render its hidden placeholder span during SSR so that the server markup matches
the first client render, which would break that assertion.

Assert what the test actually cares about instead: the portaled content is not
in the server markup, and whatever is emitted is either nothing or the hidden
placeholder. Passes both before and after that change.
@TrevorBurnham
TrevorBurnham marked this pull request as ready for review August 15, 2026 23:23
@TrevorBurnham
TrevorBurnham requested a review from a team as a code owner August 15, 2026 23:23
@TrevorBurnham
TrevorBurnham requested review from ywyyu17 and removed request for a team August 15, 2026 23:23
@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.66%. Comparing base (f066dd8) to head (99e161c).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4908   +/-   ##
=======================================
  Coverage   97.66%   97.66%           
=======================================
  Files         958      958           
  Lines       31317    31317           
  Branches    11566    11566           
=======================================
  Hits        30585    30585           
  Misses        686      686           
  Partials       46       46           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TrevorBurnham

Copy link
Copy Markdown
Contributor Author

CI run failures appear to be due to an issue on main, unrelated to the changes in this PR.

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