Skip to content

Commit 952aa74

Browse files
acdliteacdlite
authored
Upgrade tests to use react/jsx-runtime (facebook#28252)
Instead of createElement. We should have done this when we initially released jsx-runtime but better late than never. The general principle is that our tests should be written using the most up-to-date idioms that we recommend for users, except when explicitly testing an edge case or legacy behavior, like for backwards compatibility. Most of the diff is related to tweaking test output and isn't very interesting. I did have to workaround an issue related to component stacks. The component stack logic depends on shared state that lives in the React module. The problem is that most of our tests reset the React module state and re-require a fresh instance of React, React DOM, etc. However, the JSX runtime is not re-required because it's injected by the compiler as a static import. This means its copy of the shared state is no longer the same as the one used by React, causing any warning logged by the JSX runtime to not include a component stack. (This same issue also breaks string refs, but since we're removing those soon I'm not so concerned about that.) The solution I went with for now is to mock the JSX runtime with a proxy that re-requires the module on every function invocation. I don't love this but it will have to do for now. What we should really do is migrate our tests away from manually resetting the module state and use import syntax instead.
1 parent 2bc7d33 commit 952aa74

18 files changed

+290
-72
lines changed

‎babel.config.js

-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
module.exports = {
44
plugins: [
55
'@babel/plugin-syntax-jsx',
6-
'@babel/plugin-transform-react-jsx',
76
'@babel/plugin-transform-flow-strip-types',
87
['@babel/plugin-proposal-class-properties', {loose: true}],
98
'syntax-trailing-function-commas',

‎package.json

+4-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"@babel/plugin-proposal-object-rest-spread": "^7.11.0",
1515
"@babel/plugin-syntax-dynamic-import": "^7.8.3",
1616
"@babel/plugin-syntax-import-meta": "^7.10.4",
17-
"@babel/plugin-syntax-jsx": "^7.10.4",
17+
"@babel/plugin-syntax-jsx": "^7.23.3",
1818
"@babel/plugin-syntax-typescript": "^7.14.5",
1919
"@babel/plugin-transform-arrow-functions": "^7.10.4",
2020
"@babel/plugin-transform-block-scoped-functions": "^7.10.4",
@@ -27,12 +27,13 @@
2727
"@babel/plugin-transform-modules-commonjs": "^7.10.4",
2828
"@babel/plugin-transform-object-super": "^7.10.4",
2929
"@babel/plugin-transform-parameters": "^7.10.5",
30-
"@babel/plugin-transform-react-jsx-source": "^7.10.5",
30+
"@babel/plugin-transform-react-jsx": "^7.23.4",
31+
"@babel/plugin-transform-react-jsx-development": "^7.22.5",
3132
"@babel/plugin-transform-shorthand-properties": "^7.10.4",
3233
"@babel/plugin-transform-spread": "^7.11.0",
3334
"@babel/plugin-transform-template-literals": "^7.10.5",
3435
"@babel/preset-flow": "^7.10.4",
35-
"@babel/preset-react": "^7.10.4",
36+
"@babel/preset-react": "^7.23.3",
3637
"@babel/traverse": "^7.11.0",
3738
"@rollup/plugin-babel": "^6.0.3",
3839
"@rollup/plugin-commonjs": "^24.0.1",

‎packages/react-dom/src/__tests__/ReactComponent-test.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ describe('ReactComponent', () => {
471471
root.render(<X />);
472472
});
473473
}).toErrorDev(
474-
'React.createElement: type is invalid -- expected a string (for built-in components) ' +
474+
'React.jsx: type is invalid -- expected a string (for built-in components) ' +
475475
'or a class/function (for composite components) but got: undefined.',
476476
),
477477
).rejects.toThrowError(
@@ -492,7 +492,7 @@ describe('ReactComponent', () => {
492492
root.render(<Y />);
493493
});
494494
}).toErrorDev(
495-
'React.createElement: type is invalid -- expected a string (for built-in components) ' +
495+
'React.jsx: type is invalid -- expected a string (for built-in components) ' +
496496
'or a class/function (for composite components) but got: null.',
497497
),
498498
).rejects.toThrowError(
@@ -528,7 +528,7 @@ describe('ReactComponent', () => {
528528
root.render(<Foo />);
529529
});
530530
}).toErrorDev(
531-
'React.createElement: type is invalid -- expected a string (for built-in components) ' +
531+
'React.jsx: type is invalid -- expected a string (for built-in components) ' +
532532
'or a class/function (for composite components) but got: undefined.',
533533
),
534534
).rejects.toThrowError(

‎packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -1007,7 +1007,7 @@ describe('ReactDOMServerIntegration', () => {
10071007
expect(() => {
10081008
EmptyComponent = <EmptyComponent />;
10091009
}).toErrorDev(
1010-
'Warning: React.createElement: type is invalid -- expected a string ' +
1010+
'Warning: React.jsx: type is invalid -- expected a string ' +
10111011
'(for built-in components) or a class/function (for composite ' +
10121012
'components) but got: object. You likely forgot to export your ' +
10131013
"component from the file it's defined in, or you might have mixed up " +
@@ -1031,7 +1031,7 @@ describe('ReactDOMServerIntegration', () => {
10311031
expect(() => {
10321032
NullComponent = <NullComponent />;
10331033
}).toErrorDev(
1034-
'Warning: React.createElement: type is invalid -- expected a string ' +
1034+
'Warning: React.jsx: type is invalid -- expected a string ' +
10351035
'(for built-in components) or a class/function (for composite ' +
10361036
'components) but got: null.',
10371037
{withoutStack: true},
@@ -1049,7 +1049,7 @@ describe('ReactDOMServerIntegration', () => {
10491049
expect(() => {
10501050
UndefinedComponent = <UndefinedComponent />;
10511051
}).toErrorDev(
1052-
'Warning: React.createElement: type is invalid -- expected a string ' +
1052+
'Warning: React.jsx: type is invalid -- expected a string ' +
10531053
'(for built-in components) or a class/function (for composite ' +
10541054
'components) but got: undefined. You likely forgot to export your ' +
10551055
"component from the file it's defined in, or you might have mixed up " +

‎packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js

+8-2
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,10 @@ describe('ReactDeprecationWarnings', () => {
9595
}
9696
class Component extends React.Component {
9797
render() {
98-
return <RefComponent ref="refComponent" __self={this} />;
98+
return React.createElement(RefComponent, {
99+
ref: 'refComponent',
100+
__self: this,
101+
});
99102
}
100103
}
101104
expect(() => {
@@ -114,7 +117,10 @@ describe('ReactDeprecationWarnings', () => {
114117
}
115118
class Component extends React.Component {
116119
render() {
117-
return <RefComponent ref="refComponent" __self={{}} />;
120+
return React.createElement(RefComponent, {
121+
ref: 'refComponent',
122+
__self: {},
123+
});
118124
}
119125
}
120126

‎packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -1244,9 +1244,9 @@ describe('ReactIncrementalErrorHandling', () => {
12441244
</ErrorBoundary>,
12451245
);
12461246
await expect(async () => await waitForAll([])).toErrorDev([
1247-
'Warning: React.createElement: type is invalid -- expected a string',
1247+
'Warning: React.jsx: type is invalid -- expected a string',
12481248
// React retries once on error
1249-
'Warning: React.createElement: type is invalid -- expected a string',
1249+
'Warning: React.jsx: type is invalid -- expected a string',
12501250
]);
12511251
expect(ReactNoop).toMatchRenderedOutput(
12521252
<span
@@ -1295,9 +1295,9 @@ describe('ReactIncrementalErrorHandling', () => {
12951295
</ErrorBoundary>,
12961296
);
12971297
await expect(async () => await waitForAll([])).toErrorDev([
1298-
'Warning: React.createElement: type is invalid -- expected a string',
1298+
'Warning: React.jsx: type is invalid -- expected a string',
12991299
// React retries once on error
1300-
'Warning: React.createElement: type is invalid -- expected a string',
1300+
'Warning: React.jsx: type is invalid -- expected a string',
13011301
]);
13021302
expect(ReactNoop).toMatchRenderedOutput(
13031303
<span
@@ -1317,7 +1317,7 @@ describe('ReactIncrementalErrorHandling', () => {
13171317
it('recovers from uncaught reconciler errors', async () => {
13181318
const InvalidType = undefined;
13191319
expect(() => ReactNoop.render(<InvalidType />)).toErrorDev(
1320-
'Warning: React.createElement: type is invalid -- expected a string',
1320+
'Warning: React.jsx: type is invalid -- expected a string',
13211321
{withoutStack: true},
13221322
);
13231323
await waitForThrow(

‎packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,22 @@ describe('ReactFreshIntegration', () => {
8686
// eslint-disable-next-line no-new-func
8787
new Function(
8888
'global',
89+
'require',
8990
'React',
9091
'Scheduler',
9192
'exports',
9293
'$RefreshReg$',
9394
'$RefreshSig$',
9495
compiled,
95-
)(global, React, Scheduler, exportsObj, $RefreshReg$, $RefreshSig$);
96+
)(
97+
global,
98+
require,
99+
React,
100+
Scheduler,
101+
exportsObj,
102+
$RefreshReg$,
103+
$RefreshSig$,
104+
);
96105
// Module systems will register exports as a fallback.
97106
// This is useful for cases when e.g. a class is exported,
98107
// and we don't want to propagate the update beyond this module.

‎packages/react-test-renderer/src/__tests__/ReactTestRenderer-test.internal.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,7 @@ describe('ReactTestRenderer', () => {
288288
expect(() => ReactTestRenderer.create(<Foo />)).toErrorDev(
289289
'Warning: Function components cannot be given refs. Attempts ' +
290290
'to access this ref will fail. ' +
291-
'Did you mean to use React.forwardRef()?\n\n' +
292-
'Check the render method of `Foo`.\n' +
291+
'Did you mean to use React.forwardRef()?\n' +
293292
' in Bar (at **)\n' +
294293
' in Foo (at **)',
295294
);

‎packages/react/src/__tests__/ReactElement-test.js ‎packages/react/src/__tests__/ReactCreateElement-test.js

+24-32
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ let act;
1414
let React;
1515
let ReactDOMClient;
1616

17-
describe('ReactElement', () => {
17+
// NOTE: This module tests the old, "classic" JSX runtime, React.createElement.
18+
// Do not use JSX syntax in this module; call React.createElement directly.
19+
describe('ReactCreateElement', () => {
1820
let ComponentClass;
1921

2022
beforeEach(() => {
@@ -24,8 +26,6 @@ describe('ReactElement', () => {
2426

2527
React = require('react');
2628
ReactDOMClient = require('react-dom/client');
27-
// NOTE: We're explicitly not using JSX here. This is intended to test
28-
// classic JS without JSX.
2929
ComponentClass = class extends React.Component {
3030
render() {
3131
return React.createElement('div');
@@ -48,24 +48,24 @@ describe('ReactElement', () => {
4848
it('should warn when `key` is being accessed on composite element', async () => {
4949
class Child extends React.Component {
5050
render() {
51-
return <div>{this.props.key}</div>;
51+
return React.createElement('div', null, this.props.key);
5252
}
5353
}
5454
class Parent extends React.Component {
5555
render() {
56-
return (
57-
<div>
58-
<Child key="0" />
59-
<Child key="1" />
60-
<Child key="2" />
61-
</div>
56+
return React.createElement(
57+
'div',
58+
null,
59+
React.createElement(Child, {key: '0'}),
60+
React.createElement(Child, {key: '1'}),
61+
React.createElement(Child, {key: '2'}),
6262
);
6363
}
6464
}
6565
const root = ReactDOMClient.createRoot(document.createElement('div'));
6666
await expect(async () => {
6767
await act(() => {
68-
root.render(<Parent />);
68+
root.render(React.createElement(Parent));
6969
});
7070
}).toErrorDev(
7171
'Child: `key` is not a prop. Trying to access it will result ' +
@@ -76,7 +76,7 @@ describe('ReactElement', () => {
7676
});
7777

7878
it('should warn when `key` is being accessed on a host element', () => {
79-
const element = <div key="3" />;
79+
const element = React.createElement('div', {key: '3'});
8080
expect(() => void element.props.key).toErrorDev(
8181
'div: `key` is not a prop. Trying to access it will result ' +
8282
'in `undefined` being returned. If you need to access the same ' +
@@ -89,23 +89,23 @@ describe('ReactElement', () => {
8989
it('should warn when `ref` is being accessed', async () => {
9090
class Child extends React.Component {
9191
render() {
92-
return <div> {this.props.ref} </div>;
92+
return React.createElement('div', null, this.props.ref);
9393
}
9494
}
9595
class Parent extends React.Component {
9696
render() {
97-
return (
98-
<div>
99-
<Child ref={React.createRef()} />
100-
</div>
97+
return React.createElement(
98+
'div',
99+
null,
100+
React.createElement(Child, {ref: React.createRef()}),
101101
);
102102
}
103103
}
104104
const root = ReactDOMClient.createRoot(document.createElement('div'));
105105

106106
await expect(async () => {
107107
await act(() => {
108-
root.render(<Parent />);
108+
root.render(React.createElement(Parent));
109109
});
110110
}).toErrorDev(
111111
'Child: `ref` is not a prop. Trying to access it will result ' +
@@ -277,8 +277,6 @@ describe('ReactElement', () => {
277277
expect(element.props.children).toEqual([1, 2, 3]);
278278
});
279279

280-
// NOTE: We're explicitly not using JSX here. This is intended to test
281-
// classic JS without JSX.
282280
it('allows static methods to be called using the type property', () => {
283281
class StaticMethodComponentClass extends React.Component {
284282
render() {
@@ -291,16 +289,12 @@ describe('ReactElement', () => {
291289
expect(element.type.someStaticMethod()).toBe('someReturnValue');
292290
});
293291

294-
// NOTE: We're explicitly not using JSX here. This is intended to test
295-
// classic JS without JSX.
296292
it('is indistinguishable from a plain object', () => {
297293
const element = React.createElement('div', {className: 'foo'});
298294
const object = {};
299295
expect(element.constructor).toBe(object.constructor);
300296
});
301297

302-
// NOTE: We're explicitly not using JSX here. This is intended to test
303-
// classic JS without JSX.
304298
it('should use default prop value when removing a prop', async () => {
305299
class Component extends React.Component {
306300
render() {
@@ -325,8 +319,6 @@ describe('ReactElement', () => {
325319
expect(instance.props.fruit).toBe('persimmon');
326320
});
327321

328-
// NOTE: We're explicitly not using JSX here. This is intended to test
329-
// classic JS without JSX.
330322
it('should normalize props with default values', async () => {
331323
let instance;
332324
class Component extends React.Component {
@@ -354,7 +346,7 @@ describe('ReactElement', () => {
354346
it('throws when changing a prop (in dev) after element creation', async () => {
355347
class Outer extends React.Component {
356348
render() {
357-
const el = <div className="moo" />;
349+
const el = React.createElement('div', {className: 'moo'});
358350

359351
if (__DEV__) {
360352
expect(function () {
@@ -374,7 +366,7 @@ describe('ReactElement', () => {
374366
const root = ReactDOMClient.createRoot(container);
375367

376368
await act(() => {
377-
root.render(<Outer color="orange" />);
369+
root.render(React.createElement(Outer, {color: 'orange'}));
378370
});
379371
if (__DEV__) {
380372
expect(container.firstChild.className).toBe('moo');
@@ -387,7 +379,7 @@ describe('ReactElement', () => {
387379
const container = document.createElement('div');
388380
class Outer extends React.Component {
389381
render() {
390-
const el = <div>{this.props.sound}</div>;
382+
const el = React.createElement('div', null, this.props.sound);
391383

392384
if (__DEV__) {
393385
expect(function () {
@@ -405,7 +397,7 @@ describe('ReactElement', () => {
405397
Outer.defaultProps = {sound: 'meow'};
406398
const root = ReactDOMClient.createRoot(container);
407399
await act(() => {
408-
root.render(<Outer />);
400+
root.render(React.createElement(Outer));
409401
});
410402
expect(container.firstChild.textContent).toBe('meow');
411403
if (__DEV__) {
@@ -422,12 +414,12 @@ describe('ReactElement', () => {
422414
test = this;
423415
}
424416
render() {
425-
return <div />;
417+
return React.createElement('div');
426418
}
427419
}
428420
const root = ReactDOMClient.createRoot(document.createElement('div'));
429421
await act(() => {
430-
root.render(<Test value={+undefined} />);
422+
root.render(React.createElement(Test, {value: +undefined}));
431423
});
432424
expect(test.props.value).toBeNaN();
433425
});

‎packages/react/src/__tests__/ReactElementValidator-test.internal.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@
1010
'use strict';
1111

1212
// NOTE: We're explicitly not using JSX in this file. This is intended to test
13-
// classic JS without JSX.
13+
// classic React.createElement without JSX.
14+
// TODO: ^ the above note is a bit stale because there are tests in this file
15+
// that do use JSX syntax. We should port them to React.createElement, and also
16+
// confirm there's a corresponding test that uses JSX syntax.
1417

1518
let PropTypes;
1619
let React;
@@ -548,7 +551,7 @@ describe('ReactElementValidator', () => {
548551
expect(() => {
549552
void (<Foo>{[<div />]}</Foo>);
550553
}).toErrorDev(
551-
'Warning: React.createElement: type is invalid -- expected a string ' +
554+
'Warning: React.jsx: type is invalid -- expected a string ' +
552555
'(for built-in components) or a class/function (for composite ' +
553556
'components) but got: undefined. You likely forgot to export your ' +
554557
"component from the file it's defined in, or you might have mixed up " +

0 commit comments

Comments
 (0)