Skip to content

Commit 198ed66

Browse files
authored
[Suspense] Use !important to hide Suspended nodes (#15861)
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 1919206 commit 198ed66

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
@@ -1359,7 +1359,7 @@ describe('ReactFresh', () => {
13591359
const fallbackChild = container.childNodes[1];
13601360
expect(primaryChild.textContent).toBe('Content 1');
13611361
expect(primaryChild.style.color).toBe('green');
1362-
expect(primaryChild.style.display).toBe('none');
1362+
expect(primaryChild.style.display).toBe('none !important');
13631363
expect(fallbackChild.textContent).toBe('Fallback 0');
13641364
expect(fallbackChild.style.color).toBe('green');
13651365
expect(fallbackChild.style.display).toBe('');
@@ -1373,7 +1373,7 @@ describe('ReactFresh', () => {
13731373
expect(container.childNodes[1]).toBe(fallbackChild);
13741374
expect(primaryChild.textContent).toBe('Content 1');
13751375
expect(primaryChild.style.color).toBe('green');
1376-
expect(primaryChild.style.display).toBe('none');
1376+
expect(primaryChild.style.display).toBe('none !important');
13771377
expect(fallbackChild.textContent).toBe('Fallback 1');
13781378
expect(fallbackChild.style.color).toBe('green');
13791379
expect(fallbackChild.style.display).toBe('');
@@ -1397,7 +1397,7 @@ describe('ReactFresh', () => {
13971397
expect(container.childNodes[1]).toBe(fallbackChild);
13981398
expect(primaryChild.textContent).toBe('Content 1');
13991399
expect(primaryChild.style.color).toBe('red');
1400-
expect(primaryChild.style.display).toBe('none');
1400+
expect(primaryChild.style.display).toBe('none !important');
14011401
expect(fallbackChild.textContent).toBe('Fallback 1');
14021402
expect(fallbackChild.style.color).toBe('red');
14031403
expect(fallbackChild.style.display).toBe('');

0 commit comments

Comments
 (0)