Skip to content

Commit 663fe97

Browse files
sebmarkbagesebmarkbage
committed
Remove Numeric Fallback of Symbols
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 663fe97

5 files changed

Lines changed: 32 additions & 201 deletions

File tree

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

Lines changed: 0 additions & 68 deletions
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‎

Lines changed: 9 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,13 @@ describe('ReactElement.jsx', () => {
2525
beforeEach(() => {
2626
jest.resetModules();
2727

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-
3328
React = require('react');
3429
JSXRuntime = require('react/jsx-runtime');
3530
JSXDEVRuntime = require('react/jsx-dev-runtime');
3631
ReactDOM = require('react-dom');
3732
ReactTestUtils = require('react-dom/test-utils');
3833
});
3934

40-
afterEach(() => {
41-
global.Symbol = originalSymbol;
42-
});
43-
4435
it('allows static methods to be called using the type property', () => {
4536
class StaticMethodComponentClass extends React.Component {
4637
render() {
@@ -53,47 +44,6 @@ describe('ReactElement.jsx', () => {
5344
expect(element.type.someStaticMethod()).toBe('someReturnValue');
5445
});
5546

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-
9747
it('is indistinguishable from a plain object', () => {
9848
const element = JSXRuntime.jsx('div', {className: 'foo'});
9949
const object = {};
@@ -288,34 +238,22 @@ describe('ReactElement.jsx', () => {
288238
});
289239

290240
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-
311241
class Component extends React.Component {
312242
render() {
313-
return JSXRuntime.jsx('div');
243+
return JSXRuntime.jsx('div', {});
314244
}
315245
}
316246

317247
expect(React.isValidElement(JSXRuntime.jsx('div', {}))).toEqual(true);
318248
expect(React.isValidElement(JSXRuntime.jsx(Component, {}))).toEqual(true);
249+
expect(
250+
React.isValidElement(JSXRuntime.jsx(JSXRuntime.Fragment, {})),
251+
).toEqual(true);
252+
if (__DEV__) {
253+
expect(React.isValidElement(JSXDEVRuntime.jsxDEV('div', {}))).toEqual(
254+
true,
255+
);
256+
}
319257

320258
expect(React.isValidElement(null)).toEqual(false);
321259
expect(React.isValidElement(true)).toEqual(false);

‎packages/shared/ReactSymbols.js‎

Lines changed: 22 additions & 43 deletions
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‎

Lines changed: 0 additions & 15 deletions
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‎

Lines changed: 1 addition & 4 deletions
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)