Skip to content

Commit fcc3652

Browse files
committed
[Suspense] Use !important to hide Suspended nodes
Suspended nodes are hidden using an inline `display: none` style. We do this instead of removing the nodes from the DOM so that their state is preserved when they are shown again. Inline styles have the greatest specificity, but they are superseded by `!important`. To prevent an external style from overriding React's, this commit changes the hidden style to `display: none !important`. MaYBE AnDREw sHOulD JusT LEArn Css I attempted to write a unit test using `getComputedStyle` but JSDOM doesn't respect `!important`. I think our existing tests are sufficient but if we were to decide we need something more robust, I would set up an e2e test.
1 parent c403ae4 commit fcc3652

File tree

3 files changed

+11
-9
lines changed

3 files changed

+11
-9
lines changed

packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ describe('ReactDOMSuspensePlaceholder', () => {
9090
);
9191
}
9292
ReactDOM.render(<App />, container);
93-
expect(divs[0].current.style.display).toEqual('none');
94-
expect(divs[1].current.style.display).toEqual('none');
95-
expect(divs[2].current.style.display).toEqual('none');
93+
expect(divs[0].current.style.display).toEqual('none !important');
94+
expect(divs[1].current.style.display).toEqual('none !important');
95+
expect(divs[2].current.style.display).toEqual('none !important');
9696

9797
await advanceTimers(500);
9898

@@ -156,12 +156,14 @@ describe('ReactDOMSuspensePlaceholder', () => {
156156
ReactDOM.render(<App />, container);
157157
});
158158
expect(container.innerHTML).toEqual(
159-
'<span style="display: none;">Sibling</span><span style="display: none;"></span>Loading...',
159+
'<span style="display: none !important;">Sibling</span><span style=' +
160+
'"display: none !important;"></span>Loading...',
160161
);
161162

162163
act(() => setIsVisible(true));
163164
expect(container.innerHTML).toEqual(
164-
'<span style="display: none;">Sibling</span><span style="display: none;"></span>Loading...',
165+
'<span style="display: none !important;">Sibling</span><span style=' +
166+
'"display: none !important;"></span>Loading...',
165167
);
166168

167169
await advanceTimers(500);

packages/react-dom/src/client/ReactDOMHostConfig.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ export function hideInstance(instance: Instance): void {
589589
// TODO: Does this work for all element types? What about MathML? Should we
590590
// pass host context to this method?
591591
instance = ((instance: any): HTMLElement);
592-
instance.style.display = 'none';
592+
instance.style.display = 'none !important';
593593
}
594594

595595
export function hideTextInstance(textInstance: TextInstance): void {

packages/react-fresh/src/__tests__/ReactFresh-test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,7 +1357,7 @@ describe('ReactFresh', () => {
13571357
const fallbackChild = container.childNodes[1];
13581358
expect(primaryChild.textContent).toBe('Content 1');
13591359
expect(primaryChild.style.color).toBe('green');
1360-
expect(primaryChild.style.display).toBe('none');
1360+
expect(primaryChild.style.display).toBe('none !important');
13611361
expect(fallbackChild.textContent).toBe('Fallback 0');
13621362
expect(fallbackChild.style.color).toBe('green');
13631363
expect(fallbackChild.style.display).toBe('');
@@ -1371,7 +1371,7 @@ describe('ReactFresh', () => {
13711371
expect(container.childNodes[1]).toBe(fallbackChild);
13721372
expect(primaryChild.textContent).toBe('Content 1');
13731373
expect(primaryChild.style.color).toBe('green');
1374-
expect(primaryChild.style.display).toBe('none');
1374+
expect(primaryChild.style.display).toBe('none !important');
13751375
expect(fallbackChild.textContent).toBe('Fallback 1');
13761376
expect(fallbackChild.style.color).toBe('green');
13771377
expect(fallbackChild.style.display).toBe('');
@@ -1395,7 +1395,7 @@ describe('ReactFresh', () => {
13951395
expect(container.childNodes[1]).toBe(fallbackChild);
13961396
expect(primaryChild.textContent).toBe('Content 1');
13971397
expect(primaryChild.style.color).toBe('red');
1398-
expect(primaryChild.style.display).toBe('none');
1398+
expect(primaryChild.style.display).toBe('none !important');
13991399
expect(fallbackChild.textContent).toBe('Fallback 1');
14001400
expect(fallbackChild.style.color).toBe('red');
14011401
expect(fallbackChild.style.display).toBe('');

0 commit comments

Comments
 (0)