Remove Numeric Fallback of Symbols (#23348)

sebmarkbage · web-flow · commit 587e759302ef · 2022-02-23T18:08:30.000-05:00
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.
diff --git a/packages/react/src/__tests__/ReactElement-test.js b/packages/react/src/__tests__/ReactElement-test.js
@@ -15,16 +15,10 @@ let ReactTestUtils;
 
 describe('ReactElement', () => {
   let ComponentClass;
-  let originalSymbol;
 
   beforeEach(() => {
     jest.resetModules();
 
-    // Delete the native Symbol if we have one to ensure we test the
-    // unpolyfilled environment.
-    originalSymbol = global.Symbol;
-    global.Symbol = undefined;
-
     React = require('react');
     ReactDOM = require('react-dom');
     ReactTestUtils = require('react-dom/test-utils');
@@ -37,14 +31,6 @@ describe('ReactElement', () => {
     };
   });
 
-  afterEach(() => {
-    global.Symbol = originalSymbol;
-  });
-
-  it('uses the fallback value when in an environment without Symbol', () => {
-    expect((<div />).$$typeof).toBe(0xeac7);
-  });
-
   it('returns a complete element according to spec', () => {
     const element = React.createElement(ComponentClass);
     expect(element.type).toBe(ComponentClass);
@@ -294,41 +280,6 @@ describe('ReactElement', () => {
     expect(element.type.someStaticMethod()).toBe('someReturnValue');
   });
 
-  // NOTE: We're explicitly not using JSX here. This is intended to test
-  // classic JS without JSX.
-  it('identifies valid elements', () => {
-    class Component extends React.Component {
-      render() {
-        return React.createElement('div');
-      }
-    }
-
-    expect(React.isValidElement(React.createElement('div'))).toEqual(true);
-    expect(React.isValidElement(React.createElement(Component))).toEqual(true);
-
-    expect(React.isValidElement(null)).toEqual(false);
-    expect(React.isValidElement(true)).toEqual(false);
-    expect(React.isValidElement({})).toEqual(false);
-    expect(React.isValidElement('string')).toEqual(false);
-    if (!__EXPERIMENTAL__) {
-      let factory;
-      expect(() => {
-        factory = React.createFactory('div');
-      }).toWarnDev(
-        'Warning: React.createFactory() is deprecated and will be removed in a ' +
-          'future major release. Consider using JSX or use React.createElement() ' +
-          'directly instead.',
-        {withoutStack: true},
-      );
-      expect(React.isValidElement(factory)).toEqual(false);
-    }
-    expect(React.isValidElement(Component)).toEqual(false);
-    expect(React.isValidElement({type: 'div', props: {}})).toEqual(false);
-
-    const jsonElement = JSON.stringify(React.createElement('div'));
-    expect(React.isValidElement(JSON.parse(jsonElement))).toBe(true);
-  });
-
   // NOTE: We're explicitly not using JSX here. This is intended to test
   // classic JS without JSX.
   it('is indistinguishable from a plain object', () => {
@@ -447,25 +398,6 @@ describe('ReactElement', () => {
   // NOTE: We're explicitly not using JSX here. This is intended to test
   // classic JS without JSX.
   it('identifies elements, but not JSON, if Symbols are supported', () => {
-    // Rudimentary polyfill
-    // Once all jest engines support Symbols natively we can swap this to test
-    // WITH native Symbols by default.
-    const REACT_ELEMENT_TYPE = function() {}; // fake Symbol
-    const OTHER_SYMBOL = function() {}; // another fake Symbol
-    global.Symbol = function(name) {
-      return OTHER_SYMBOL;
-    };
-    global.Symbol.for = function(key) {
-      if (key === 'react.element') {
-        return REACT_ELEMENT_TYPE;
-      }
-      return OTHER_SYMBOL;
-    };
-
-    jest.resetModules();
-
-    React = require('react');
-
     class Component extends React.Component {
       render() {
         return React.createElement('div');
diff --git a/packages/react/src/__tests__/ReactElementJSX-test.js b/packages/react/src/__tests__/ReactElementJSX-test.js
@@ -20,27 +20,16 @@ let JSXDEVRuntime;
 // A lot of these tests are pulled from ReactElement-test because
 // this api is meant to be backwards compatible.
 describe('ReactElement.jsx', () => {
-  let originalSymbol;
-
   beforeEach(() => {
     jest.resetModules();
 
-    // Delete the native Symbol if we have one to ensure we test the
-    // unpolyfilled environment.
-    originalSymbol = global.Symbol;
-    global.Symbol = undefined;
-
     React = require('react');
     JSXRuntime = require('react/jsx-runtime');
     JSXDEVRuntime = require('react/jsx-dev-runtime');
     ReactDOM = require('react-dom');
     ReactTestUtils = require('react-dom/test-utils');
   });
 
-  afterEach(() => {
-    global.Symbol = originalSymbol;
-  });
-
   it('allows static methods to be called using the type property', () => {
     class StaticMethodComponentClass extends React.Component {
       render() {
@@ -53,47 +42,6 @@ describe('ReactElement.jsx', () => {
     expect(element.type.someStaticMethod()).toBe('someReturnValue');
   });
 
-  it('identifies valid elements', () => {
-    class Component extends React.Component {
-      render() {
-        return JSXRuntime.jsx('div', {});
-      }
-    }
-
-    expect(React.isValidElement(JSXRuntime.jsx('div', {}))).toEqual(true);
-    expect(React.isValidElement(JSXRuntime.jsx(Component, {}))).toEqual(true);
-    expect(
-      React.isValidElement(JSXRuntime.jsx(JSXRuntime.Fragment, {})),
-    ).toEqual(true);
-    if (__DEV__) {
-      expect(React.isValidElement(JSXDEVRuntime.jsxDEV('div', {}))).toEqual(
-        true,
-      );
-    }
-
-    expect(React.isValidElement(null)).toEqual(false);
-    expect(React.isValidElement(true)).toEqual(false);
-    expect(React.isValidElement({})).toEqual(false);
-    expect(React.isValidElement('string')).toEqual(false);
-    if (!__EXPERIMENTAL__) {
-      let factory;
-      expect(() => {
-        factory = React.createFactory('div');
-      }).toWarnDev(
-        'Warning: React.createFactory() is deprecated and will be removed in a ' +
-          'future major release. Consider using JSX or use React.createElement() ' +
-          'directly instead.',
-        {withoutStack: true},
-      );
-      expect(React.isValidElement(factory)).toEqual(false);
-    }
-    expect(React.isValidElement(Component)).toEqual(false);
-    expect(React.isValidElement({type: 'div', props: {}})).toEqual(false);
-
-    const jsonElement = JSON.stringify(JSXRuntime.jsx('div', {}));
-    expect(React.isValidElement(JSON.parse(jsonElement))).toBe(true);
-  });
-
   it('is indistinguishable from a plain object', () => {
     const element = JSXRuntime.jsx('div', {className: 'foo'});
     const object = {};
@@ -288,34 +236,22 @@ describe('ReactElement.jsx', () => {
   });
 
   it('identifies elements, but not JSON, if Symbols are supported', () => {
-    // Rudimentary polyfill
-    // Once all jest engines support Symbols natively we can swap this to test
-    // WITH native Symbols by default.
-    const REACT_ELEMENT_TYPE = function() {}; // fake Symbol
-    const OTHER_SYMBOL = function() {}; // another fake Symbol
-    global.Symbol = function(name) {
-      return OTHER_SYMBOL;
-    };
-    global.Symbol.for = function(key) {
-      if (key === 'react.element') {
-        return REACT_ELEMENT_TYPE;
-      }
-      return OTHER_SYMBOL;
-    };
-
-    jest.resetModules();
-
-    React = require('react');
-    JSXRuntime = require('react/jsx-runtime');
-
     class Component extends React.Component {
       render() {
-        return JSXRuntime.jsx('div');
+        return JSXRuntime.jsx('div', {});
       }
     }
 
     expect(React.isValidElement(JSXRuntime.jsx('div', {}))).toEqual(true);
     expect(React.isValidElement(JSXRuntime.jsx(Component, {}))).toEqual(true);
+    expect(
+      React.isValidElement(JSXRuntime.jsx(JSXRuntime.Fragment, {})),
+    ).toEqual(true);
+    if (__DEV__) {
+      expect(React.isValidElement(JSXDEVRuntime.jsxDEV('div', {}))).toEqual(
+        true,
+      );
+    }
 
     expect(React.isValidElement(null)).toEqual(false);
     expect(React.isValidElement(true)).toEqual(false);
diff --git a/packages/shared/ReactSymbols.js b/packages/shared/ReactSymbols.js
@@ -11,50 +11,29 @@
 // When adding new symbols to this file,
 // Please consider also adding to 'react-devtools-shared/src/backend/ReactSymbols'
 
-// The Symbol used to tag the ReactElement-like types. If there is no native Symbol
-// nor polyfill, then a plain number is used for performance.
-export let REACT_ELEMENT_TYPE = 0xeac7;
-export let REACT_PORTAL_TYPE = 0xeaca;
-export let REACT_FRAGMENT_TYPE = 0xeacb;
-export let REACT_STRICT_MODE_TYPE = 0xeacc;
-export let REACT_PROFILER_TYPE = 0xead2;
-export let REACT_PROVIDER_TYPE = 0xeacd;
-export let REACT_CONTEXT_TYPE = 0xeace;
-export let REACT_FORWARD_REF_TYPE = 0xead0;
-export let REACT_SUSPENSE_TYPE = 0xead1;
-export let REACT_SUSPENSE_LIST_TYPE = 0xead8;
-export let REACT_MEMO_TYPE = 0xead3;
-export let REACT_LAZY_TYPE = 0xead4;
-export let REACT_SCOPE_TYPE = 0xead7;
-export let REACT_DEBUG_TRACING_MODE_TYPE = 0xeae1;
-export let REACT_OFFSCREEN_TYPE = 0xeae2;
-export let REACT_LEGACY_HIDDEN_TYPE = 0xeae3;
-export let REACT_CACHE_TYPE = 0xeae4;
-export let REACT_TRACING_MARKER_TYPE = 0xeae5;
+// The Symbol used to tag the ReactElement-like types.
+export const REACT_ELEMENT_TYPE = Symbol.for('react.element');
+export const REACT_PORTAL_TYPE = Symbol.for('react.portal');
+export const REACT_FRAGMENT_TYPE = Symbol.for('react.fragment');
+export const REACT_STRICT_MODE_TYPE = Symbol.for('react.strict_mode');
+export const REACT_PROFILER_TYPE = Symbol.for('react.profiler');
+export const REACT_PROVIDER_TYPE = Symbol.for('react.provider');
+export const REACT_CONTEXT_TYPE = Symbol.for('react.context');
+export const REACT_FORWARD_REF_TYPE = Symbol.for('react.forward_ref');
+export const REACT_SUSPENSE_TYPE = Symbol.for('react.suspense');
+export const REACT_SUSPENSE_LIST_TYPE = Symbol.for('react.suspense_list');
+export const REACT_MEMO_TYPE = Symbol.for('react.memo');
+export const REACT_LAZY_TYPE = Symbol.for('react.lazy');
+export const REACT_SCOPE_TYPE = Symbol.for('react.scope');
+export const REACT_DEBUG_TRACING_MODE_TYPE = Symbol.for(
+  'react.debug_trace_mode',
+);
+export const REACT_OFFSCREEN_TYPE = Symbol.for('react.offscreen');
+export const REACT_LEGACY_HIDDEN_TYPE = Symbol.for('react.legacy_hidden');
+export const REACT_CACHE_TYPE = Symbol.for('react.cache');
+export const REACT_TRACING_MARKER_TYPE = Symbol.for('react.tracing_marker');
 
-if (typeof Symbol === 'function' && Symbol.for) {
-  const symbolFor = Symbol.for;
-  REACT_ELEMENT_TYPE = symbolFor('react.element');
-  REACT_PORTAL_TYPE = symbolFor('react.portal');
-  REACT_FRAGMENT_TYPE = symbolFor('react.fragment');
-  REACT_STRICT_MODE_TYPE = symbolFor('react.strict_mode');
-  REACT_PROFILER_TYPE = symbolFor('react.profiler');
-  REACT_PROVIDER_TYPE = symbolFor('react.provider');
-  REACT_CONTEXT_TYPE = symbolFor('react.context');
-  REACT_FORWARD_REF_TYPE = symbolFor('react.forward_ref');
-  REACT_SUSPENSE_TYPE = symbolFor('react.suspense');
-  REACT_SUSPENSE_LIST_TYPE = symbolFor('react.suspense_list');
-  REACT_MEMO_TYPE = symbolFor('react.memo');
-  REACT_LAZY_TYPE = symbolFor('react.lazy');
-  REACT_SCOPE_TYPE = symbolFor('react.scope');
-  REACT_DEBUG_TRACING_MODE_TYPE = symbolFor('react.debug_trace_mode');
-  REACT_OFFSCREEN_TYPE = symbolFor('react.offscreen');
-  REACT_LEGACY_HIDDEN_TYPE = symbolFor('react.legacy_hidden');
-  REACT_CACHE_TYPE = symbolFor('react.cache');
-  REACT_TRACING_MARKER_TYPE = symbolFor('react.tracing_marker');
-}
-
-const MAYBE_ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator;
+const MAYBE_ITERATOR_SYMBOL = Symbol.iterator;
 const FAUX_ITERATOR_SYMBOL = '@@iterator';
 
 export function getIteratorFn(maybeIterable: ?any): ?() => ?Iterator<*> {
diff --git a/packages/shared/__tests__/ReactSymbols-test.internal.js b/packages/shared/__tests__/ReactSymbols-test.internal.js
@@ -26,19 +26,4 @@ describe('ReactSymbols', () => {
   it('Symbol values should be unique', () => {
     expectToBeUnique(Object.entries(require('shared/ReactSymbols')));
   });
-
-  it('numeric values should be unique', () => {
-    const originalSymbolFor = global.Symbol.for;
-    global.Symbol.for = null;
-    try {
-      const entries = Object.entries(require('shared/ReactSymbols')).filter(
-        // REACT_ASYNC_MODE_TYPE and REACT_CONCURRENT_MODE_TYPE have the same numeric value
-        // for legacy backwards compatibility
-        ([key]) => key !== 'REACT_ASYNC_MODE_TYPE',
-      );
-      expectToBeUnique(entries);
-    } finally {
-      global.Symbol.for = originalSymbolFor;
-    }
-  });
 });
diff --git a/packages/shared/isValidElementType.js b/packages/shared/isValidElementType.js
@@ -31,10 +31,7 @@ import {
   enableTransitionTracing,
 } from './ReactFeatureFlags';
 
-let REACT_MODULE_REFERENCE: number | Symbol = 0;
-if (typeof Symbol === 'function') {
-  REACT_MODULE_REFERENCE = Symbol.for('react.module.reference');
-}
+const REACT_MODULE_REFERENCE: Symbol = Symbol.for('react.module.reference');
 
 export default function isValidElementType(type: mixed) {
   if (typeof type === 'string' || typeof type === 'function') {
