Skip to content

[EuiPortal] Migrate to function component#9598

Merged
mgadewoll merged 4 commits intoelastic:mainfrom
mgadewoll:portal/migrate-to-function-component
Apr 28, 2026
Merged

[EuiPortal] Migrate to function component#9598
mgadewoll merged 4 commits intoelastic:mainfrom
mgadewoll:portal/migrate-to-function-component

Conversation

@mgadewoll
Copy link
Copy Markdown
Contributor

@mgadewoll mgadewoll commented Apr 17, 2026

Summary

closes #9456
closes #8784

This PR refactors EuiPortal from a class component to a function component.

API Changes

⚪ no API changes

Screenshots

description Before After
DOM output Screenshot 2026-04-17 at 13 48 19 Screenshot 2026-04-17 at 13 48 53
DOM output in strict mode
Screen.Recording.2026-04-17.at.13.51.42.mov
Screen.Recording.2026-04-17.at.13.52.13.mov

Impact Assessment

Note: Most PRs should be tested in Kibana to help gauge their Impact before merging.

  • 🔴 Breaking changes — What will break? How many usages in Kibana/Cloud UI are impacted?
  • 💅 Visual changes — May impact style overrides; could require visual testing. Explain and estimate impact.
  • 🧪 Test impact — May break functional or snapshot tests (e.g., HTML structure, class names, default values).
  • 🔧 Hard to integrate — If changes require substantial updates to Kibana, please stage the changes and link them here.

Impact level: 🟢 None

The refactor is a 1:1 functional and structural migration. There should not be any impact.

ℹ️ The changes were run in Kibana CI (🟢 CI build)

Release Readiness

  • Documentation: {link to docs page(s)}
  • Figma: {link to Figma or issue}
  • Migration guide: {steps or link, for breaking/visual changes or deprecations}
  • Adoption plan (new features): {link to issue/doc or outline who will integrate this and where}

QA instructions for reviewer

💻 EuiPortal storybook
💻 React 18 strict mode testing codesandbox
💻 React 19 strict mode testing codesandbox

  • verify there is no regression with current production
  • verify the component works as expected in strict mode (e.g. use the above linked codesandboxes or enable it locally in Storybook by adding strictMode: true here or spin up your own react app)
  • verify that components using EuiPortal work as expected (see above linked codesandboxes)

Checklist before marking Ready for Review

Reviewer checklist

  • Approved Impact Assessment — Acceptable to merge given the consumer impact.
  • Approved Release Readiness — Docs, Figma, and migration info are sufficient to ship.

@mgadewoll mgadewoll self-assigned this Apr 17, 2026
@mgadewoll mgadewoll added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Apr 17, 2026
@mgadewoll mgadewoll marked this pull request as ready for review April 17, 2026 13:13
@mgadewoll mgadewoll requested a review from a team as a code owner April 17, 2026 13:13
Copilot AI review requested due to automatic review settings April 17, 2026 13:13
Copy link
Copy Markdown
Contributor

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

Refactors EuiPortal (core utility used by overlays) from a class component to a hook-based function component, aiming to preserve existing portal insertion/removal behavior and improve React Strict Mode cleanup behavior.

Changes:

  • Converted EuiPortal from a class component to a memoized function component using useState/useContext.
  • Replaced componentDidMount/componentWillUnmount with an isomorphic useLayoutEffect/useEffect pattern to create/cleanup the portal node.
  • Removed the previously exported EuiPortalClass implementation.

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

Comment thread packages/eui/src/components/portal/portal.tsx
Comment on lines +49 to +51
export const EuiPortal: FunctionComponent<EuiPortalProps> = memo((_props) => {
const props = usePropsWithComponentDefaults('EuiPortal', _props);
const { children, insert, portalRef } = props;
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

This refactor removes the previously exported EuiPortalClass symbol from this module. Even if it wasn't re-exported from components/portal/index.ts, consumers can still deep-import it from components/portal/portal, so this is a potential breaking API change and also conflicts with the PR description of “no API changes”. Either keep a backwards-compatible export (e.g., a deprecated alias) or explicitly call this out as an API/breaking change.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The class component was exported from the file, but not from the portal folder.
If someone imported it manually, it would be rather unexpected usage. The only officially exported component was EuiPortal. Imho, this doesn't require marking it as breaking change.

@tkajtoch tkajtoch self-requested a review April 20, 2026 12:45
Copy link
Copy Markdown
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

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

Code changes look great! I tested portalled components with and without StrictMode, and used the provided CodeSandbox links to test the same in React 19. Everything works as expected, and the attached Kibana CI run assures me that everything's safe.

@mgadewoll
Copy link
Copy Markdown
Contributor Author

mgadewoll commented Apr 28, 2026

ℹ️ To be on the safe side I'll run it in Kibana CI (CI run) again, considering that the last run has been some time ago already.

@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

History

cc @mgadewoll

@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

History

cc @mgadewoll

@mgadewoll
Copy link
Copy Markdown
Contributor Author

ℹ️ To be on the safe side I'll run it in Kibana CI (CI run) again, considering that the last run has been some time ago already.

The CI run finished 🟢

@mgadewoll mgadewoll merged commit b02ce61 into elastic:main Apr 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[EuiPortal] Migrate from class to function component [Strict Mode] Redundant div is portalled and not cleaned up

4 participants