From bde12f6ababf2a1b214b0cfebcbc103ec012a338 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 9 Apr 2020 20:25:44 -0700 Subject: [PATCH] Add destructuring to force toObject which throws before the side-effects This ensures that we don't double call yieldValue or advanceTime in tests. Ideally we could use empty destructuring but ES lint doesn't like it. --- ...DOMServerIntegrationHooks-test.internal.js | 4 +- .../ReactErrorBoundaries-test.internal.js | 8 +- ...tIncrementalErrorHandling-test.internal.js | 76 +++++++++---------- .../__tests__/ReactProfiler-test.internal.js | 4 +- .../src/__tests__/forwardRef-test.internal.js | 3 +- 5 files changed, 48 insertions(+), 47 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js index a910b011f0507..359c61a2fada8 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js @@ -1097,7 +1097,7 @@ describe('ReactDOMServerHooks', () => { it('useOpaqueIdentifier: ID is not used during hydration but is used in an update', async () => { let _setShow; - function App() { + function App({unused}) { Scheduler.unstable_yieldValue('App'); const id = useOpaqueIdentifier(); const [show, setShow] = useState(false); @@ -1129,7 +1129,7 @@ describe('ReactDOMServerHooks', () => { it('useOpaqueIdentifier: ID is not used during hydration but is used in an update in legacy', async () => { let _setShow; - function App() { + function App({unused}) { Scheduler.unstable_yieldValue('App'); const id = useOpaqueIdentifier(); const [show, setShow] = useState(false); diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 7601a47230973..81427c97eb055 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -497,7 +497,7 @@ describe('ReactErrorBoundaries', () => { } }; - BrokenUseEffect = props => { + BrokenUseEffect = ({children}) => { Scheduler.unstable_yieldValue('BrokenUseEffect render'); React.useEffect(() => { @@ -505,10 +505,10 @@ describe('ReactErrorBoundaries', () => { throw new Error('Hello'); }); - return props.children; + return children; }; - BrokenUseLayoutEffect = props => { + BrokenUseLayoutEffect = ({children}) => { Scheduler.unstable_yieldValue('BrokenUseLayoutEffect render'); React.useLayoutEffect(() => { @@ -518,7 +518,7 @@ describe('ReactErrorBoundaries', () => { throw new Error('Hello'); }); - return props.children; + return children; }; NoopErrorBoundary = class extends React.Component { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 8ab0ba3ed247c..b9b5836e70960 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -62,17 +62,17 @@ describe('ReactIncrementalErrorHandling', () => { } } - function ErrorMessage(props) { + function ErrorMessage({error}) { Scheduler.unstable_yieldValue('ErrorMessage'); - return ; + return ; } - function Indirection(props) { + function Indirection({children}) { Scheduler.unstable_yieldValue('Indirection'); - return props.children || null; + return children || null; } - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('throw'); throw new Error('oops!'); } @@ -156,17 +156,17 @@ describe('ReactIncrementalErrorHandling', () => { } } - function ErrorMessage(props) { + function ErrorMessage({error}) { Scheduler.unstable_yieldValue('ErrorMessage'); - return ; + return ; } - function Indirection(props) { + function Indirection({children}) { Scheduler.unstable_yieldValue('Indirection'); - return props.children || null; + return children || null; } - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('throw'); throw new Error('oops!'); } @@ -346,17 +346,17 @@ describe('ReactIncrementalErrorHandling', () => { }); it('retries one more time before handling error', () => { - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('BadRender'); throw new Error('oops'); } - function Sibling() { + function Sibling({unused}) { Scheduler.unstable_yieldValue('Sibling'); return ; } - function Parent() { + function Parent({unused}) { Scheduler.unstable_yieldValue('Parent'); return ( <> @@ -386,7 +386,7 @@ describe('ReactIncrementalErrorHandling', () => { }); it('retries one more time if an error occurs during a render that expires midway through the tree', () => { - function Oops() { + function Oops({unused}) { Scheduler.unstable_yieldValue('Oops'); throw new Error('Oops'); } @@ -396,7 +396,7 @@ describe('ReactIncrementalErrorHandling', () => { return text; } - function App() { + function App({unused}) { return ( <> @@ -530,7 +530,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender(props) { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -576,7 +576,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender(props) { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -623,7 +623,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender(props) { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -664,7 +664,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -703,7 +703,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -744,7 +744,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -784,7 +784,7 @@ describe('ReactIncrementalErrorHandling', () => { } } - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } @@ -862,12 +862,12 @@ describe('ReactIncrementalErrorHandling', () => { }); it('can schedule updates after uncaught error in render on mount', () => { - function BrokenRender() { + function BrokenRender({unused}) { Scheduler.unstable_yieldValue('BrokenRender'); throw new Error('Hello'); } - function Foo() { + function Foo({unused}) { Scheduler.unstable_yieldValue('Foo'); return null; } @@ -887,24 +887,24 @@ describe('ReactIncrementalErrorHandling', () => { }); it('can schedule updates after uncaught error in render on update', () => { - function BrokenRender(props) { + function BrokenRender({shouldThrow}) { Scheduler.unstable_yieldValue('BrokenRender'); - if (props.throw) { + if (shouldThrow) { throw new Error('Hello'); } return null; } - function Foo() { + function Foo({unused}) { Scheduler.unstable_yieldValue('Foo'); return null; } - ReactNoop.render(); + ReactNoop.render(); expect(Scheduler).toFlushAndYield(['BrokenRender']); expect(() => { - ReactNoop.render(); + ReactNoop.render(); expect(Scheduler).toFlushWithoutYielding(); }).toThrow('Hello'); expect(Scheduler).toHaveYielded([ @@ -1454,13 +1454,13 @@ describe('ReactIncrementalErrorHandling', () => { } } - function Indirection(props) { + function Indirection({children}) { Scheduler.unstable_yieldValue('Indirection'); - return props.children; + return children; } const notAnError = {nonStandardMessage: 'oops'}; - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('BadRender'); throw notAnError; } @@ -1519,17 +1519,17 @@ describe('ReactIncrementalErrorHandling', () => { } } - function ErrorMessage(props) { + function ErrorMessage({error}) { Scheduler.unstable_yieldValue('ErrorMessage'); - return ; + return ; } - function BadRenderSibling(props) { + function BadRenderSibling({unused}) { Scheduler.unstable_yieldValue('BadRenderSibling'); return null; } - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('throw'); throw new Error('oops!'); } @@ -1567,7 +1567,7 @@ describe('ReactIncrementalErrorHandling', () => { // This test seems a bit contrived, but it's based on an actual regression // where we checked for the existence of didUpdate instead of didMount, and // didMount was not defined. - function BadRender() { + function BadRender({unused}) { Scheduler.unstable_yieldValue('throw'); throw new Error('oops!'); } @@ -1711,7 +1711,7 @@ describe('ReactIncrementalErrorHandling', () => { it('uncaught errors should be discarded if the render is aborted', async () => { const root = ReactNoop.createRoot(); - function Oops() { + function Oops({unused}) { Scheduler.unstable_yieldValue('Oops'); throw Error('Oops'); } diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 1fce1caa623cf..b3019fc280e55 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -1041,7 +1041,7 @@ describe('Profiler', () => { it('should accumulate actual time after an error handled by componentDidCatch()', () => { const callback = jest.fn(); - const ThrowsError = () => { + const ThrowsError = ({unused}) => { Scheduler.unstable_advanceTime(3); throw Error('expected error'); }; @@ -1120,7 +1120,7 @@ describe('Profiler', () => { it('should accumulate actual time after an error handled by getDerivedStateFromError()', () => { const callback = jest.fn(); - const ThrowsError = () => { + const ThrowsError = ({unused}) => { Scheduler.unstable_advanceTime(10); throw Error('expected error'); }; diff --git a/packages/react/src/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js index d98dfabcad671..ef8c7422f87aa 100644 --- a/packages/react/src/__tests__/forwardRef-test.internal.js +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -159,8 +159,9 @@ describe('forwardRef', () => { } function Wrapper(props) { + const forwardedRef = props.forwardedRef; Scheduler.unstable_yieldValue('Wrapper'); - return ; + return ; } const RefForwardingComponent = React.forwardRef((props, ref) => (