Skip to content

Commit

Permalink
Add disableLegacyContext test gates where needed (#26371)
Browse files Browse the repository at this point in the history
The www builds include disableLegacyContext as a dynamic flag, so we
should be running the tests in that mode, too. Previously we were
overriding the flag during the test run. This strategy usually doesn't
work because the flags get compiled out in the final build, but we
happen to not test www in build mode, only source.

To get of this hacky override, I added a test gate to every test that
uses legacy context. When we eventually remove legacy context from the
codebase, this should make it slightly easier to find which tests are
affected. And removes one more hack from our hack-ridden test config.

Given that sometimes www has features enabled that aren't on in other
builds, we might want to consider testing its build artifacts in CI,
rather than just source. That would have forced this cleanup to happen
sooner. Currently we only test the public builds in CI.
  • Loading branch information
acdlite authored Mar 11, 2023
1 parent 432ffc9 commit 6334614
Show file tree
Hide file tree
Showing 21 changed files with 326 additions and 245 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ describe('ReactCompositeComponent', () => {
);
});

// @gate !disableLegacyContext
it('should pass context to children when not owner', () => {
class Parent extends React.Component {
render() {
Expand Down Expand Up @@ -652,6 +653,7 @@ describe('ReactCompositeComponent', () => {
expect(childRenders).toBe(1);
});

// @gate !disableLegacyContext
it('should pass context when re-rendered for static child', () => {
let parentInstance = null;
let childInstance = null;
Expand Down Expand Up @@ -712,6 +714,7 @@ describe('ReactCompositeComponent', () => {
expect(childInstance.context).toEqual({foo: 'bar', flag: true});
});

// @gate !disableLegacyContext
it('should pass context when re-rendered for static child within a composite component', () => {
class Parent extends React.Component {
static childContextTypes = {
Expand Down Expand Up @@ -768,6 +771,7 @@ describe('ReactCompositeComponent', () => {
expect(wrapper.childRef.current.context).toEqual({flag: false});
});

// @gate !disableLegacyContext
it('should pass context transitively', () => {
let childInstance = null;
let grandchildInstance = null;
Expand Down Expand Up @@ -829,6 +833,7 @@ describe('ReactCompositeComponent', () => {
expect(grandchildInstance.context).toEqual({foo: 'bar', depth: 1});
});

// @gate !disableLegacyContext
it('should pass context when re-rendered', () => {
let parentInstance = null;
let childInstance = null;
Expand Down Expand Up @@ -883,6 +888,7 @@ describe('ReactCompositeComponent', () => {
expect(childInstance.context).toEqual({foo: 'bar', depth: 0});
});

// @gate !disableLegacyContext
it('unmasked context propagates through updates', () => {
class Leaf extends React.Component {
static contextTypes = {
Expand Down Expand Up @@ -946,6 +952,7 @@ describe('ReactCompositeComponent', () => {
expect(div.children[0].id).toBe('aliens');
});

// @gate !disableLegacyContext
it('should trigger componentWillReceiveProps for context changes', () => {
let contextChanges = 0;
let propChanges = 0;
Expand Down Expand Up @@ -1219,6 +1226,7 @@ describe('ReactCompositeComponent', () => {
expect(a).toBe(b);
});

// @gate !disableLegacyContext || !__DEV__
it('context should be passed down from the parent', () => {
class Parent extends React.Component {
static childContextTypes = {
Expand Down
3 changes: 3 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFiber-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,7 @@ describe('ReactDOMFiber', () => {
);
});

// @gate !disableLegacyContext
it('should pass portal context when rendering subtree elsewhere', () => {
const portalContainer = document.createElement('div');

Expand Down Expand Up @@ -752,6 +753,7 @@ describe('ReactDOMFiber', () => {
expect(portalContainer.innerHTML).toBe('<div>bar</div>');
});

// @gate !disableLegacyContext
it('should update portal context if it changes due to setState', () => {
const portalContainer = document.createElement('div');

Expand Down Expand Up @@ -796,6 +798,7 @@ describe('ReactDOMFiber', () => {
expect(container.innerHTML).toBe('');
});

// @gate !disableLegacyContext
it('should update portal context if it changes due to re-render', () => {
const portalContainer = document.createElement('div');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1624,6 +1624,7 @@ describe('ReactDOMFizzServer', () => {
}
});

// @gate !disableLegacyContext
it('should can suspend in a class component with legacy context', async () => {
class TestProvider extends React.Component {
static childContextTypes = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ describe('ReactDOMServerIntegration', () => {
});

describe('legacy context', function () {
// The `itRenders` test abstraction doesn't work with @gate so we have
// to do this instead.
if (gate(flags => flags.disableLegacyContext)) {
test('empty test to stop Jest from being a complainy complainer', () => {});
return;
}

let PurpleContext, RedContext;
beforeEach(() => {
class Parent extends React.Component {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,7 @@ describe('ReactErrorBoundaries', () => {
assertLog(['ErrorBoundary componentWillUnmount']);
});

// @gate !disableLegacyContext || !__DEV__
it('renders an error state if context provider throws in componentWillMount', () => {
class BrokenComponentWillMountWithContext extends React.Component {
static childContextTypes = {foo: PropTypes.number};
Expand All @@ -799,45 +800,45 @@ describe('ReactErrorBoundaries', () => {
expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
});

if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) {
it('renders an error state if module-style context provider throws in componentWillMount', () => {
function BrokenComponentWillMountWithContext() {
return {
getChildContext() {
return {foo: 42};
},
render() {
return <div>{this.props.children}</div>;
},
UNSAFE_componentWillMount() {
throw new Error('Hello');
},
};
}
BrokenComponentWillMountWithContext.childContextTypes = {
foo: PropTypes.number,
// @gate !disableModulePatternComponents
// @gate !disableLegacyContext
it('renders an error state if module-style context provider throws in componentWillMount', () => {
function BrokenComponentWillMountWithContext() {
return {
getChildContext() {
return {foo: 42};
},
render() {
return <div>{this.props.children}</div>;
},
UNSAFE_componentWillMount() {
throw new Error('Hello');
},
};
}
BrokenComponentWillMountWithContext.childContextTypes = {
foo: PropTypes.number,
};

const container = document.createElement('div');
expect(() =>
ReactDOM.render(
<ErrorBoundary>
<BrokenComponentWillMountWithContext />
</ErrorBoundary>,
container,
),
).toErrorDev(
'Warning: The <BrokenComponentWillMountWithContext /> component appears to be a function component that ' +
'returns a class instance. ' +
'Change BrokenComponentWillMountWithContext to a class that extends React.Component instead. ' +
"If you can't use a class try assigning the prototype on the function as a workaround. " +
'`BrokenComponentWillMountWithContext.prototype = React.Component.prototype`. ' +
"Don't use an arrow function since it cannot be called with `new` by React.",
);
const container = document.createElement('div');
expect(() =>
ReactDOM.render(
<ErrorBoundary>
<BrokenComponentWillMountWithContext />
</ErrorBoundary>,
container,
),
).toErrorDev(
'Warning: The <BrokenComponentWillMountWithContext /> component appears to be a function component that ' +
'returns a class instance. ' +
'Change BrokenComponentWillMountWithContext to a class that extends React.Component instead. ' +
"If you can't use a class try assigning the prototype on the function as a workaround. " +
'`BrokenComponentWillMountWithContext.prototype = React.Component.prototype`. ' +
"Don't use an arrow function since it cannot be called with `new` by React.",
);

expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
});
}
expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
});

it('mounts the error message if mounting fails', () => {
function renderError(error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ describe('ReactFunctionComponent', () => {
expect(container.textContent).toBe('');
});

// @gate !disableLegacyContext
it('should pass context thru stateless component', () => {
class Child extends React.Component {
static contextTypes = {
Expand Down Expand Up @@ -305,6 +306,7 @@ describe('ReactFunctionComponent', () => {

// This guards against a regression caused by clearing the current debug fiber.
// https://github.com/facebook/react/issues/10831
// @gate !disableLegacyContext || !__DEV__
it('should warn when giving a function ref with context', () => {
function Child() {
return null;
Expand Down Expand Up @@ -375,6 +377,7 @@ describe('ReactFunctionComponent', () => {
]);
});

// @gate !disableLegacyContext
it('should receive context', () => {
class Parent extends React.Component {
static childContextTypes = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,7 @@ describe('ReactLegacyErrorBoundaries', () => {
expect(log).toEqual(['ErrorBoundary componentWillUnmount']);
});

// @gate !disableLegacyContext || !__DEV__
it('renders an error state if context provider throws in componentWillMount', () => {
class BrokenComponentWillMountWithContext extends React.Component {
static childContextTypes = {foo: PropTypes.number};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ describe('ReactDOMServer', () => {
expect(markup).toContain('hello, world');
});

// @gate !disableLegacyContext
it('renders with context when using custom constructor', () => {
class Component extends React.Component {
constructor() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const renderSubtreeIntoContainer =
require('react-dom').unstable_renderSubtreeIntoContainer;

describe('renderSubtreeIntoContainer', () => {
// @gate !disableLegacyContext
it('should pass context when rendering subtree elsewhere', () => {
const portal = document.createElement('div');

Expand Down Expand Up @@ -99,6 +100,7 @@ describe('renderSubtreeIntoContainer', () => {
}
});

// @gate !disableLegacyContext
it('should update context if it changes due to setState', () => {
const container = document.createElement('div');
document.body.appendChild(container);
Expand Down Expand Up @@ -159,6 +161,7 @@ describe('renderSubtreeIntoContainer', () => {
expect(portal.firstChild.innerHTML).toBe('changed-changed');
});

// @gate !disableLegacyContext
it('should update context if it changes due to re-render', () => {
const container = document.createElement('div');
document.body.appendChild(container);
Expand Down Expand Up @@ -238,6 +241,7 @@ describe('renderSubtreeIntoContainer', () => {
expect(portal.firstChild.innerHTML).toBe('hello');
});

// @gate !disableLegacyContext
it('should get context through non-context-provider parent', () => {
const container = document.createElement('div');
document.body.appendChild(container);
Expand Down Expand Up @@ -281,6 +285,7 @@ describe('renderSubtreeIntoContainer', () => {
expect(portal.textContent).toBe('foo');
});

// @gate !disableLegacyContext
it('should get context through middle non-context-provider layer', () => {
const container = document.createElement('div');
document.body.appendChild(container);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ it('handles events', () => {
]);
});

// @gate !disableLegacyContext || !__DEV__
it('handles events on text nodes', () => {
expect(RCTEventEmitter.register).toHaveBeenCalledTimes(1);
const EventEmitter = RCTEventEmitter.register.mock.calls[0][0];
Expand Down
Loading

0 comments on commit 6334614

Please sign in to comment.