Skip to content

Commit 587e759

Browse files
sebmarkbagesebmarkbage
authored
Remove Numeric Fallback of Symbols (#23348)
This was already defeating the XSS issue that Symbols was meant to protect against. So you were already supposed to use a polyfill for security. We rely on real Symbol.for in Flight for Server Components so those require real symbols anyway. We also don't really support IE without additional polyfills anyway.
1 parent 4035157 commit 587e759

File tree

5 files changed

+32
-203
lines changed

5 files changed

+32
-203
lines changed

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

-68
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,10 @@ let ReactTestUtils;
1515

1616
describe('ReactElement', () => {
1717
let ComponentClass;
18-
let originalSymbol;
1918

2019
beforeEach(() => {
2120
jest.resetModules();
2221

23-
// Delete the native Symbol if we have one to ensure we test the
24-
// unpolyfilled environment.
25-
originalSymbol = global.Symbol;
26-
global.Symbol = undefined;
27-
2822
React = require('react');
2923
ReactDOM = require('react-dom');
3024
ReactTestUtils = require('react-dom/test-utils');
@@ -37,14 +31,6 @@ describe('ReactElement', () => {
3731
};
3832
});
3933

40-
afterEach(() => {
41-
global.Symbol = originalSymbol;
42-
});
43-
44-
it('uses the fallback value when in an environment without Symbol', () => {
45-
expect((<div />).$$typeof).toBe(0xeac7);
46-
});
47-
4834
it('returns a complete element according to spec', () => {
4935
const element = React.createElement(ComponentClass);
5036
expect(element.type).toBe(ComponentClass);
@@ -294,41 +280,6 @@ describe('ReactElement', () => {
294280
expect(element.type.someStaticMethod()).toBe('someReturnValue');
295281
});
296282

297-
// NOTE: We're explicitly not using JSX here. This is intended to test
298-
// classic JS without JSX.
299-
it('identifies valid elements', () => {
300-
class Component extends React.Component {
301-
render() {
302-
return React.createElement('div');
303-
}
304-
}
305-
306-
expect(React.isValidElement(React.createElement('div'))).toEqual(true);
307-
expect(React.isValidElement(React.createElement(Component))).toEqual(true);
308-
309-
expect(React.isValidElement(null)).toEqual(false);
310-
expect(React.isValidElement(true)).toEqual(false);
311-
expect(React.isValidElement({})).toEqual(false);
312-
expect(React.isValidElement('string')).toEqual(false);
313-
if (!__EXPERIMENTAL__) {
314-
let factory;
315-
expect(() => {
316-
factory = React.createFactory('div');
317-
}).toWarnDev(
318-
'Warning: React.createFactory() is deprecated and will be removed in a ' +
319-
'future major release. Consider using JSX or use React.createElement() ' +
320-
'directly instead.',
321-
{withoutStack: true},
322-
);
323-
expect(React.isValidElement(factory)).toEqual(false);
324-
}
325-
expect(React.isValidElement(Component)).toEqual(false);
326-
expect(React.isValidElement({type: 'div', props: {}})).toEqual(false);
327-
328-
const jsonElement = JSON.stringify(React.createElement('div'));
329-
expect(React.isValidElement(JSON.parse(jsonElement))).toBe(true);
330-
});
331-
332283
// NOTE: We're explicitly not using JSX here. This is intended to test
333284
// classic JS without JSX.
334285
it('is indistinguishable from a plain object', () => {
@@ -447,25 +398,6 @@ describe('ReactElement', () => {
447398
// NOTE: We're explicitly not using JSX here. This is intended to test
448399
// classic JS without JSX.
449400
it('identifies elements, but not JSON, if Symbols are supported', () => {
450-
// Rudimentary polyfill
451-
// Once all jest engines support Symbols natively we can swap this to test
452-
// WITH native Symbols by default.
453-
const REACT_ELEMENT_TYPE = function() {}; // fake Symbol
454-
const OTHER_SYMBOL = function() {}; // another fake Symbol
455-
global.Symbol = function(name) {
456-
return OTHER_SYMBOL;
457-
};
458-
global.Symbol.for = function(key) {
459-
if (key === 'react.element') {
460-
return REACT_ELEMENT_TYPE;
461-
}
462-
return OTHER_SYMBOL;
463-
};
464-
465-
jest.resetModules();
466-
467-
React = require('react');
468-
469401
class Component extends React.Component {
470402
render() {
471403
return React.createElement('div');

‎packages/react/src/__tests__/ReactElementJSX-test.js

+9-73
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,16 @@ let JSXDEVRuntime;
2020
// A lot of these tests are pulled from ReactElement-test because
2121
// this api is meant to be backwards compatible.
2222
describe('ReactElement.jsx', () => {
23-
let originalSymbol;
24-
2523
beforeEach(() => {
2624
jest.resetModules();
2725

28-
// Delete the native Symbol if we have one to ensure we test the
29-
// unpolyfilled environment.
30-
originalSymbol = global.Symbol;
31-
global.Symbol = undefined;
32-
3326
React = require('react');
3427
JSXRuntime = require('react/jsx-runtime');
3528
JSXDEVRuntime = require('react/jsx-dev-runtime');
3629
ReactDOM = require('react-dom');
3730
ReactTestUtils = require('react-dom/test-utils');
3831
});
3932

40-
afterEach(() => {
41-
global.Symbol = originalSymbol;
42-
});
43-
4433
it('allows static methods to be called using the type property', () => {
4534
class StaticMethodComponentClass extends React.Component {
4635
render() {
@@ -53,47 +42,6 @@ describe('ReactElement.jsx', () => {
5342
expect(element.type.someStaticMethod()).toBe('someReturnValue');
5443
});
5544

56-
it('identifies valid elements', () => {
57-
class Component extends React.Component {
58-
render() {
59-
return JSXRuntime.jsx('div', {});
60-
}
61-
}
62-
63-
expect(React.isValidElement(JSXRuntime.jsx('div', {}))).toEqual(true);
64-
expect(React.isValidElement(JSXRuntime.jsx(Component, {}))).toEqual(true);
65-
expect(
66-
React.isValidElement(JSXRuntime.jsx(JSXRuntime.Fragment, {})),
67-
).toEqual(true);
68-
if (__DEV__) {
69-
expect(React.isValidElement(JSXDEVRuntime.jsxDEV('div', {}))).toEqual(
70-
true,
71-
);
72-
}
73-
74-
expect(React.isValidElement(null)).toEqual(false);
75-
expect(React.isValidElement(true)).toEqual(false);
76-
expect(React.isValidElement({})).toEqual(false);
77-
expect(React.isValidElement('string')).toEqual(false);
78-
if (!__EXPERIMENTAL__) {
79-
let factory;
80-
expect(() => {
81-
factory = React.createFactory('div');
82-
}).toWarnDev(
83-
'Warning: React.createFactory() is deprecated and will be removed in a ' +
84-
'future major release. Consider using JSX or use React.createElement() ' +
85-
'directly instead.',
86-
{withoutStack: true},
87-
);
88-
expect(React.isValidElement(factory)).toEqual(false);
89-
}
90-
expect(React.isValidElement(Component)).toEqual(false);
91-
expect(React.isValidElement({type: 'div', props: {}})).toEqual(false);
92-
93-
const jsonElement = JSON.stringify(JSXRuntime.jsx('div', {}));
94-
expect(React.isValidElement(JSON.parse(jsonElement))).toBe(true);
95-
});
96-
9745
it('is indistinguishable from a plain object', () => {
9846
const element = JSXRuntime.jsx('div', {className: 'foo'});
9947
const object = {};
@@ -288,34 +236,22 @@ describe('ReactElement.jsx', () => {
288236
});
289237

290238
it('identifies elements, but not JSON, if Symbols are supported', () => {
291-
// Rudimentary polyfill
292-
// Once all jest engines support Symbols natively we can swap this to test
293-
// WITH native Symbols by default.
294-
const REACT_ELEMENT_TYPE = function() {}; // fake Symbol
295-
const OTHER_SYMBOL = function() {}; // another fake Symbol
296-
global.Symbol = function(name) {
297-
return OTHER_SYMBOL;
298-
};
299-
global.Symbol.for = function(key) {
300-
if (key === 'react.element') {
301-
return REACT_ELEMENT_TYPE;
302-
}
303-
return OTHER_SYMBOL;
304-
};
305-
306-
jest.resetModules();
307-
308-
React = require('react');
309-
JSXRuntime = require('react/jsx-runtime');
310-
311239
class Component extends React.Component {
312240
render() {
313-
return JSXRuntime.jsx('div');
241+
return JSXRuntime.jsx('div', {});
314242
}
315243
}
316244

317245
expect(React.isValidElement(JSXRuntime.jsx('div', {}))).toEqual(true);
318246
expect(React.isValidElement(JSXRuntime.jsx(Component, {}))).toEqual(true);
247+
expect(
248+
React.isValidElement(JSXRuntime.jsx(JSXRuntime.Fragment, {})),
249+
).toEqual(true);
250+
if (__DEV__) {
251+
expect(React.isValidElement(JSXDEVRuntime.jsxDEV('div', {}))).toEqual(
252+
true,
253+
);
254+
}
319255

320256
expect(React.isValidElement(null)).toEqual(false);
321257
expect(React.isValidElement(true)).toEqual(false);

‎packages/shared/ReactSymbols.js

+22-43
Original file line numberDiff line numberDiff line change
@@ -11,50 +11,29 @@
1111
// When adding new symbols to this file,
1212
// Please consider also adding to 'react-devtools-shared/src/backend/ReactSymbols'
1313

14-
// The Symbol used to tag the ReactElement-like types. If there is no native Symbol
15-
// nor polyfill, then a plain number is used for performance.
16-
export let REACT_ELEMENT_TYPE = 0xeac7;
17-
export let REACT_PORTAL_TYPE = 0xeaca;
18-
export let REACT_FRAGMENT_TYPE = 0xeacb;
19-
export let REACT_STRICT_MODE_TYPE = 0xeacc;
20-
export let REACT_PROFILER_TYPE = 0xead2;
21-
export let REACT_PROVIDER_TYPE = 0xeacd;
22-
export let REACT_CONTEXT_TYPE = 0xeace;
23-
export let REACT_FORWARD_REF_TYPE = 0xead0;
24-
export let REACT_SUSPENSE_TYPE = 0xead1;
25-
export let REACT_SUSPENSE_LIST_TYPE = 0xead8;
26-
export let REACT_MEMO_TYPE = 0xead3;
27-
export let REACT_LAZY_TYPE = 0xead4;
28-
export let REACT_SCOPE_TYPE = 0xead7;
29-
export let REACT_DEBUG_TRACING_MODE_TYPE = 0xeae1;
30-
export let REACT_OFFSCREEN_TYPE = 0xeae2;
31-
export let REACT_LEGACY_HIDDEN_TYPE = 0xeae3;
32-
export let REACT_CACHE_TYPE = 0xeae4;
33-
export let REACT_TRACING_MARKER_TYPE = 0xeae5;
14+
// The Symbol used to tag the ReactElement-like types.
15+
export const REACT_ELEMENT_TYPE = Symbol.for('react.element');
16+
export const REACT_PORTAL_TYPE = Symbol.for('react.portal');
17+
export const REACT_FRAGMENT_TYPE = Symbol.for('react.fragment');
18+
export const REACT_STRICT_MODE_TYPE = Symbol.for('react.strict_mode');
19+
export const REACT_PROFILER_TYPE = Symbol.for('react.profiler');
20+
export const REACT_PROVIDER_TYPE = Symbol.for('react.provider');
21+
export const REACT_CONTEXT_TYPE = Symbol.for('react.context');
22+
export const REACT_FORWARD_REF_TYPE = Symbol.for('react.forward_ref');
23+
export const REACT_SUSPENSE_TYPE = Symbol.for('react.suspense');
24+
export const REACT_SUSPENSE_LIST_TYPE = Symbol.for('react.suspense_list');
25+
export const REACT_MEMO_TYPE = Symbol.for('react.memo');
26+
export const REACT_LAZY_TYPE = Symbol.for('react.lazy');
27+
export const REACT_SCOPE_TYPE = Symbol.for('react.scope');
28+
export const REACT_DEBUG_TRACING_MODE_TYPE = Symbol.for(
29+
'react.debug_trace_mode',
30+
);
31+
export const REACT_OFFSCREEN_TYPE = Symbol.for('react.offscreen');
32+
export const REACT_LEGACY_HIDDEN_TYPE = Symbol.for('react.legacy_hidden');
33+
export const REACT_CACHE_TYPE = Symbol.for('react.cache');
34+
export const REACT_TRACING_MARKER_TYPE = Symbol.for('react.tracing_marker');
3435

35-
if (typeof Symbol === 'function' && Symbol.for) {
36-
const symbolFor = Symbol.for;
37-
REACT_ELEMENT_TYPE = symbolFor('react.element');
38-
REACT_PORTAL_TYPE = symbolFor('react.portal');
39-
REACT_FRAGMENT_TYPE = symbolFor('react.fragment');
40-
REACT_STRICT_MODE_TYPE = symbolFor('react.strict_mode');
41-
REACT_PROFILER_TYPE = symbolFor('react.profiler');
42-
REACT_PROVIDER_TYPE = symbolFor('react.provider');
43-
REACT_CONTEXT_TYPE = symbolFor('react.context');
44-
REACT_FORWARD_REF_TYPE = symbolFor('react.forward_ref');
45-
REACT_SUSPENSE_TYPE = symbolFor('react.suspense');
46-
REACT_SUSPENSE_LIST_TYPE = symbolFor('react.suspense_list');
47-
REACT_MEMO_TYPE = symbolFor('react.memo');
48-
REACT_LAZY_TYPE = symbolFor('react.lazy');
49-
REACT_SCOPE_TYPE = symbolFor('react.scope');
50-
REACT_DEBUG_TRACING_MODE_TYPE = symbolFor('react.debug_trace_mode');
51-
REACT_OFFSCREEN_TYPE = symbolFor('react.offscreen');
52-
REACT_LEGACY_HIDDEN_TYPE = symbolFor('react.legacy_hidden');
53-
REACT_CACHE_TYPE = symbolFor('react.cache');
54-
REACT_TRACING_MARKER_TYPE = symbolFor('react.tracing_marker');
55-
}
56-
57-
const MAYBE_ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator;
36+
const MAYBE_ITERATOR_SYMBOL = Symbol.iterator;
5837
const FAUX_ITERATOR_SYMBOL = '@@iterator';
5938

6039
export function getIteratorFn(maybeIterable: ?any): ?() => ?Iterator<*> {

‎packages/shared/__tests__/ReactSymbols-test.internal.js

-15
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,4 @@ describe('ReactSymbols', () => {
2626
it('Symbol values should be unique', () => {
2727
expectToBeUnique(Object.entries(require('shared/ReactSymbols')));
2828
});
29-
30-
it('numeric values should be unique', () => {
31-
const originalSymbolFor = global.Symbol.for;
32-
global.Symbol.for = null;
33-
try {
34-
const entries = Object.entries(require('shared/ReactSymbols')).filter(
35-
// REACT_ASYNC_MODE_TYPE and REACT_CONCURRENT_MODE_TYPE have the same numeric value
36-
// for legacy backwards compatibility
37-
([key]) => key !== 'REACT_ASYNC_MODE_TYPE',
38-
);
39-
expectToBeUnique(entries);
40-
} finally {
41-
global.Symbol.for = originalSymbolFor;
42-
}
43-
});
4429
});

‎packages/shared/isValidElementType.js

+1-4
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,7 @@ import {
3131
enableTransitionTracing,
3232
} from './ReactFeatureFlags';
3333

34-
let REACT_MODULE_REFERENCE: number | Symbol = 0;
35-
if (typeof Symbol === 'function') {
36-
REACT_MODULE_REFERENCE = Symbol.for('react.module.reference');
37-
}
34+
const REACT_MODULE_REFERENCE: Symbol = Symbol.for('react.module.reference');
3835

3936
export default function isValidElementType(type: mixed) {
4037
if (typeof type === 'string' || typeof type === 'function') {

0 commit comments

Comments
 (0)