Skip to content

Commit

Permalink
Add destructuring to force toObject which throws before the side-effects
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@sebmarkbage
sebmarkbage committed Apr 10, 2020
1 parent 1352994 commit bde12f6
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 47 deletions.
4 changes: 2 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js
View file
Expand Up @@ -497,18 +497,18 @@ describe('ReactErrorBoundaries', () => {
}
};

BrokenUseEffect = props => {
BrokenUseEffect = ({children}) => {
Scheduler.unstable_yieldValue('BrokenUseEffect render');

React.useEffect(() => {
Scheduler.unstable_yieldValue('BrokenUseEffect useEffect [!]');
throw new Error('Hello');
});

return props.children;
return children;
};

BrokenUseLayoutEffect = props => {
BrokenUseLayoutEffect = ({children}) => {
Scheduler.unstable_yieldValue('BrokenUseLayoutEffect render');

React.useLayoutEffect(() => {
Expand All @@ -518,7 +518,7 @@ describe('ReactErrorBoundaries', () => {
throw new Error('Hello');
});

return props.children;
return children;
};

NoopErrorBoundary = class extends React.Component {
Expand Down
76 changes: 38 additions & 38 deletions packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
View file
Expand Up @@ -62,17 +62,17 @@ describe('ReactIncrementalErrorHandling', () => {
}
}

function ErrorMessage(props) {
function ErrorMessage({error}) {
Scheduler.unstable_yieldValue('ErrorMessage');
return <span prop={`Caught an error: ${props.error.message}`} />;
return <span prop={`Caught an error: ${error.message}`} />;
}

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!');
}
Expand Down Expand Up @@ -156,17 +156,17 @@ describe('ReactIncrementalErrorHandling', () => {
}
}

function ErrorMessage(props) {
function ErrorMessage({error}) {
Scheduler.unstable_yieldValue('ErrorMessage');
return <span prop={`Caught an error: ${props.error.message}`} />;
return <span prop={`Caught an error: ${error.message}`} />;
}

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!');
}
Expand Down Expand Up @@ -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 <span prop="Sibling" />;
}

function Parent() {
function Parent({unused}) {
Scheduler.unstable_yieldValue('Parent');
return (
<>
Expand Down Expand Up @@ -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');
}
Expand All @@ -396,7 +396,7 @@ describe('ReactIncrementalErrorHandling', () => {
return text;
}

function App() {
function App({unused}) {
return (
<>
<Text text="A" />
Expand Down Expand Up @@ -530,7 +530,7 @@ describe('ReactIncrementalErrorHandling', () => {
}
}

function BrokenRender(props) {
function BrokenRender({unused}) {
Scheduler.unstable_yieldValue('BrokenRender');
throw new Error('Hello');
}
Expand Down Expand Up @@ -576,7 +576,7 @@ describe('ReactIncrementalErrorHandling', () => {
}
}

function BrokenRender(props) {
function BrokenRender({unused}) {
Scheduler.unstable_yieldValue('BrokenRender');
throw new Error('Hello');
}
Expand Down Expand Up @@ -623,7 +623,7 @@ describe('ReactIncrementalErrorHandling', () => {
}
}

function BrokenRender(props) {
function BrokenRender({unused}) {
Scheduler.unstable_yieldValue('BrokenRender');
throw new Error('Hello');
}
Expand Down Expand Up @@ -664,7 +664,7 @@ describe('ReactIncrementalErrorHandling', () => {
}
}

function BrokenRender() {
function BrokenRender({unused}) {
Scheduler.unstable_yieldValue('BrokenRender');
throw new Error('Hello');
}
Expand Down Expand Up @@ -703,7 +703,7 @@ describe('ReactIncrementalErrorHandling', () => {
}
}

function BrokenRender() {
function BrokenRender({unused}) {
Scheduler.unstable_yieldValue('BrokenRender');
throw new Error('Hello');
}
Expand Down Expand Up @@ -744,7 +744,7 @@ describe('ReactIncrementalErrorHandling', () => {
}
}

function BrokenRender() {
function BrokenRender({unused}) {
Scheduler.unstable_yieldValue('BrokenRender');
throw new Error('Hello');
}
Expand Down Expand Up @@ -784,7 +784,7 @@ describe('ReactIncrementalErrorHandling', () => {
}
}

function BrokenRender() {
function BrokenRender({unused}) {
Scheduler.unstable_yieldValue('BrokenRender');
throw new Error('Hello');
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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(<BrokenRender throw={false} />);
ReactNoop.render(<BrokenRender shouldThrow={false} />);
expect(Scheduler).toFlushAndYield(['BrokenRender']);

expect(() => {
ReactNoop.render(<BrokenRender throw={true} />);
ReactNoop.render(<BrokenRender shouldThrow={true} />);
expect(Scheduler).toFlushWithoutYielding();
}).toThrow('Hello');
expect(Scheduler).toHaveYielded([
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -1519,17 +1519,17 @@ describe('ReactIncrementalErrorHandling', () => {
}
}

function ErrorMessage(props) {
function ErrorMessage({error}) {
Scheduler.unstable_yieldValue('ErrorMessage');
return <span prop={`Caught an error: ${props.error.message}`} />;
return <span prop={`Caught an error: ${error.message}`} />;
}

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!');
}
Expand Down Expand Up @@ -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!');
}
Expand Down Expand Up @@ -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');
}
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/__tests__/ReactProfiler-test.internal.js
View file
Expand Up @@ -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');
};
Expand Down Expand Up @@ -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');
};
Expand Down
3 changes: 2 additions & 1 deletion packages/react/src/__tests__/forwardRef-test.internal.js
View file
Expand Up @@ -159,8 +159,9 @@ describe('forwardRef', () => {
}

function Wrapper(props) {
const forwardedRef = props.forwardedRef;
Scheduler.unstable_yieldValue('Wrapper');
return <BadRender {...props} ref={props.forwardedRef} />;
return <BadRender {...props} ref={forwardedRef} />;
}

const RefForwardingComponent = React.forwardRef((props, ref) => (
Expand Down

0 comments on commit bde12f6

Please sign in to comment.