Skip to content

Commit

Permalink
Revert "Remove Numeric Fallback of Symbols (facebook#23348)"
Browse files Browse the repository at this point in the history
This reverts commit 587e759.
  • Loading branch information
salazarm committed Mar 17, 2022
1 parent 1093750 commit f04f670
Show file tree
Hide file tree
Showing 5 changed files with 203 additions and 36 deletions.
68 changes: 68 additions & 0 deletions packages/react/src/__tests__/ReactElement-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,16 @@ 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');
Expand All @@ -31,6 +37,14 @@ 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);
Expand Down Expand Up @@ -280,6 +294,41 @@ 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', () => {
Expand Down Expand Up @@ -398,6 +447,25 @@ 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');
Expand Down
82 changes: 73 additions & 9 deletions packages/react/src/__tests__/ReactElementJSX-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,27 @@ 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() {
Expand All @@ -42,6 +53,47 @@ 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 = {};
Expand Down Expand Up @@ -236,22 +288,34 @@ 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);
Expand Down
69 changes: 43 additions & 26 deletions packages/shared/ReactSymbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,33 +11,50 @@
// 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.
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_SERVER_CONTEXT_TYPE = Symbol.for('react.server_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');
export const REACT_SERVER_CONTEXT_DEFAULT_VALUE_NOT_LOADED = Symbol.for(
'react.default_value',
);
// 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;

const MAYBE_ITERATOR_SYMBOL = Symbol.iterator;
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 FAUX_ITERATOR_SYMBOL = '@@iterator';

export function getIteratorFn(maybeIterable: ?any): ?() => ?Iterator<*> {
Expand Down
15 changes: 15 additions & 0 deletions packages/shared/__tests__/ReactSymbols-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,19 @@ 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;
}
});
});
5 changes: 4 additions & 1 deletion packages/shared/isValidElementType.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ import {
enableLegacyHidden,
} from './ReactFeatureFlags';

const REACT_MODULE_REFERENCE: Symbol = Symbol.for('react.module.reference');
let REACT_MODULE_REFERENCE: number | Symbol = 0;
if (typeof Symbol === 'function') {
REACT_MODULE_REFERENCE = Symbol.for('react.module.reference');
}

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

0 comments on commit f04f670

Please sign in to comment.