-
Notifications
You must be signed in to change notification settings - Fork 2.9k
refactor(react-portal): remove Griffel dependency from usePortalMountNode #35994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
7004446
refactor(react-portal): remove Griffel dependency from usePortalMount…
dmytrokirpa 0f72ebc
change
dmytrokirpa 5f0ad5f
fix linter issue
dmytrokirpa 281bde8
update class attribute handling to keep the test snapshots without ch…
dmytrokirpa dc95f82
fix(react-portal): avoid WeakMap memory leak by not storing HTMLStyle…
Copilot cc8a5b7
refactor(react-portal): use Symbol on document for ref count, insertR…
Copilot f00a291
refactor(react-portal): Symbol.for cross-bundle, DocumentWithPortalCo…
Copilot a306576
refactor(react-portal): move types above constants, rename doc to tar…
Copilot 95a0658
test(react-portal): add unit tests for usePortalMountNodeStyles hook
Copilot 398ae03
refactor(react-portal): use id instead of data attribute for style el…
Copilot 79e95f5
refactor(react-portal): export getPortalRefCount/setPortalRefCount an…
Copilot 162a0c0
fix react 17 integration tests
dmytrokirpa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
7 changes: 7 additions & 0 deletions
7
change/@fluentui-react-portal-ea64a7c1-d032-4c04-a5b8-083f1685c723.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "patch", | ||
| "comment": "refactor: remove Griffel dependency from usePortalMountNode", | ||
| "packageName": "@fluentui/react-portal", | ||
| "email": "dmytrokirpa@microsoft.com", | ||
| "dependentChangeType": "patch" | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
18 changes: 0 additions & 18 deletions
18
...-components/react-portal/library/src/components/Portal/usePortalMountNodeStyles.styles.ts
This file was deleted.
Oops, something went wrong.
97 changes: 97 additions & 0 deletions
97
...t-components/react-portal/library/src/components/Portal/usePortalMountNodeStyles.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| import { renderHook } from '@testing-library/react-hooks'; | ||
| import { Provider_unstable as Provider } from '@fluentui/react-shared-contexts'; | ||
| import * as React from 'react'; | ||
|
|
||
| import { usePortalMountNodeStyles, PORTAL_STYLE_ELEMENT_ID, setPortalRefCount } from './usePortalMountNodeStyles'; | ||
|
|
||
| function queryStyleElement(): HTMLStyleElement | null { | ||
| return document.head.querySelector(`#${PORTAL_STYLE_ELEMENT_ID}`); | ||
| } | ||
|
|
||
| function createWrapper(targetDocument: Document | undefined) { | ||
| return (props: { children?: React.ReactNode }) => ( | ||
| <Provider value={{ dir: 'ltr', targetDocument }}>{props.children}</Provider> | ||
| ); | ||
| } | ||
|
|
||
| describe('usePortalMountNodeStyles', () => { | ||
| afterEach(() => { | ||
| // Clean up any leftover style elements and reset the ref count | ||
| queryStyleElement()?.remove(); | ||
| setPortalRefCount(document, 0); | ||
| }); | ||
|
|
||
| it('injects a <style> element into document.head when enabled', () => { | ||
| expect(queryStyleElement()).toBeNull(); | ||
|
|
||
| renderHook(() => usePortalMountNodeStyles(false)); | ||
|
|
||
| const style = queryStyleElement(); | ||
| expect(style).not.toBeNull(); | ||
| expect(style!.parentElement).toBe(document.head); | ||
| }); | ||
|
|
||
| it('does not inject a <style> element when disabled', () => { | ||
| renderHook(() => usePortalMountNodeStyles(true)); | ||
|
|
||
| expect(queryStyleElement()).toBeNull(); | ||
| }); | ||
|
|
||
| it('does not inject a <style> element when targetDocument is undefined', () => { | ||
| renderHook(() => usePortalMountNodeStyles(false), { | ||
| wrapper: createWrapper(undefined), | ||
| }); | ||
|
|
||
| expect(queryStyleElement()).toBeNull(); | ||
| }); | ||
|
|
||
| it('removes the <style> element on unmount', () => { | ||
| const { unmount } = renderHook(() => usePortalMountNodeStyles(false)); | ||
|
|
||
| expect(queryStyleElement()).not.toBeNull(); | ||
|
|
||
| unmount(); | ||
|
|
||
| expect(queryStyleElement()).toBeNull(); | ||
| }); | ||
|
|
||
| it('shares a single <style> element across multiple consumers', () => { | ||
| const hook1 = renderHook(() => usePortalMountNodeStyles(false)); | ||
| const hook2 = renderHook(() => usePortalMountNodeStyles(false)); | ||
|
|
||
| const allStyles = document.head.querySelectorAll(`#${PORTAL_STYLE_ELEMENT_ID}`); | ||
| expect(allStyles.length).toBe(1); | ||
|
|
||
| // Unmounting one consumer keeps the style | ||
| hook1.unmount(); | ||
| expect(queryStyleElement()).not.toBeNull(); | ||
|
|
||
| // Unmounting the last consumer removes it | ||
| hook2.unmount(); | ||
| expect(queryStyleElement()).toBeNull(); | ||
| }); | ||
|
|
||
| it('injects the style rule via insertRule', () => { | ||
| renderHook(() => usePortalMountNodeStyles(false)); | ||
|
|
||
| const style = queryStyleElement(); | ||
| expect(style).not.toBeNull(); | ||
| expect(style!.sheet).not.toBeNull(); | ||
| expect(style!.sheet!.cssRules.length).toBe(1); | ||
| expect(style!.sheet!.cssRules[0].cssText).toContain('[data-portal-node]'); | ||
| }); | ||
|
|
||
| it('prepends the <style> element as the first child of head', () => { | ||
| // Add an existing element to head | ||
| const existing = document.createElement('link'); | ||
| document.head.appendChild(existing); | ||
|
|
||
| renderHook(() => usePortalMountNodeStyles(false)); | ||
|
|
||
| const style = queryStyleElement(); | ||
| expect(style).not.toBeNull(); | ||
| expect(document.head.firstElementChild).toBe(style); | ||
|
|
||
| existing.remove(); | ||
| }); | ||
| }); |
83 changes: 83 additions & 0 deletions
83
...s/react-components/react-portal/library/src/components/Portal/usePortalMountNodeStyles.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| 'use client'; | ||
|
|
||
| import * as React from 'react'; | ||
| import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; | ||
| import { useIsomorphicLayoutEffect } from '@fluentui/react-utilities'; | ||
|
|
||
| // String concatenation is used to prevent bundlers to complain with older versions of React | ||
| const useInsertionEffect = (React as never)['useInsertion' + 'Effect'] | ||
| ? (React as never)['useInsertion' + 'Effect'] | ||
| : useIsomorphicLayoutEffect; | ||
|
|
||
| // Symbol used as a "private" property key on Document to store the active portal reference count. | ||
| // Symbol.for() registers in the global Symbol registry so the same key is shared across bundles | ||
| // (e.g. when multiple copies of this module are loaded in the same page). | ||
| // Storing state directly on the document avoids any WeakMap cross-reference issues and is safe | ||
| // across multiple documents (e.g. iframes) because each document object carries its own counter. | ||
| const PORTAL_STYLE_REF_COUNT = Symbol.for('fui-portal-style-ref-count'); | ||
|
|
||
| type DocumentWithPortalCounter = Document & { [PORTAL_STYLE_REF_COUNT]?: number }; | ||
|
|
||
| // Creates new stacking context to prevent z-index issues | ||
| // https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_context | ||
| // | ||
| // Also keeps a portal on top of a page to prevent scrollbars from appearing | ||
| const PORTAL_MOUNT_NODE_STYLE_RULE = `[data-portal-node]{position:absolute;top:0;left:0;right:0;z-index:1000000}`; | ||
|
|
||
| // ID used to identify the injected portal mount node <style> element in a document. | ||
| // Only one such element exists per document, so an id is appropriate. | ||
| export const PORTAL_STYLE_ELEMENT_ID = 'fui-portal-styles'; | ||
|
|
||
| export function getPortalRefCount(targetDocument: Document): number { | ||
| return (targetDocument as DocumentWithPortalCounter)[PORTAL_STYLE_REF_COUNT] ?? 0; | ||
| } | ||
|
|
||
| export function setPortalRefCount(targetDocument: Document, count: number): void { | ||
| (targetDocument as DocumentWithPortalCounter)[PORTAL_STYLE_REF_COUNT] = count; | ||
| } | ||
|
|
||
| function injectPortalMountNodeStyles(targetDocument: Document): void { | ||
| const currentCount = getPortalRefCount(targetDocument); | ||
| if (currentCount > 0) { | ||
| setPortalRefCount(targetDocument, currentCount + 1); | ||
| return; | ||
| } | ||
| const style = targetDocument.createElement('style'); | ||
| style.id = PORTAL_STYLE_ELEMENT_ID; | ||
| // Prepend so that consumer class names (applied later in document order) can override these | ||
| // defaults via CSS source order at equal specificity — the same cascade behaviour as before. | ||
| // Both prepend and append trigger one style recalculation; position in <head> does not change | ||
| // the number of recalcs. | ||
| targetDocument.head.prepend(style); | ||
| // sheet is available after the element is inserted into the document | ||
| style.sheet?.insertRule(PORTAL_MOUNT_NODE_STYLE_RULE); | ||
| setPortalRefCount(targetDocument, 1); | ||
| } | ||
|
|
||
| function ejectPortalMountNodeStyles(targetDocument: Document): void { | ||
| const currentCount = getPortalRefCount(targetDocument); | ||
| if (currentCount === 0) { | ||
| return; | ||
| } | ||
| const newCount = currentCount - 1; | ||
| if (newCount === 0) { | ||
| targetDocument.head.querySelector(`#${PORTAL_STYLE_ELEMENT_ID}`)?.remove(); | ||
| } | ||
| setPortalRefCount(targetDocument, newCount); | ||
| } | ||
|
|
||
| /** | ||
| * Injects a shared <style> element for portal mount node styling into the target document, | ||
| * and removes it when the last consumer unmounts (reference counted via a Symbol property on `document`). | ||
| */ | ||
| export function usePortalMountNodeStyles(disabled: boolean | undefined): void { | ||
| const { targetDocument } = useFluent(); | ||
|
|
||
| useInsertionEffect!(() => { | ||
| if (disabled || !targetDocument) { | ||
| return; | ||
| } | ||
| injectPortalMountNodeStyles(targetDocument); | ||
| return () => ejectPortalMountNodeStyles(targetDocument); | ||
| }, [disabled, targetDocument]); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.