Improve DEV errors if string coercion throws (Temporal.*, Symbol, etc.) (#22064)

* Revise ESLint rules for string coercion

Currently, react uses `'' + value` to coerce mixed values to strings.
This code will throw for Temporal objects or symbols.

To make string-coercion safer and to improve user-facing error messages,
This commit adds a new ESLint rule called `safe-string-coercion`.

This rule has two modes: a production mode and a non-production mode.
* If the `isProductionUserAppCode` option is true, then `'' + value`
  coercions are allowed (because they're faster, although they may
  throw) and `String(value)` coercions are disallowed. Exception:
  when building error messages or running DEV-only code in prod
  files, `String()` should be used because it won't throw.
* If the `isProductionUserAppCode` option is false, then `'' + value`
  coercions are disallowed (because they may throw, and in non-prod
  code it's not worth the risk) and `String(value)` are allowed.

Production mode is used for all files which will be bundled with
developers' userland apps. Non-prod mode is used for all other React
code: tests, DEV blocks, devtools extension, etc.

In production mode, in addiiton to flagging `String(value)` calls,
the rule will also flag `'' + value` or `value + ''` coercions that may
throw. The rule is smart enough to silence itself in the following
"will never throw" cases:
* When the coercion is wrapped in a `typeof` test that restricts to safe
  (non-symbol, non-object) types. Example:
    if (typeof value === 'string' || typeof value === 'number') {
      thisWontReport('' + value);
    }
* When what's being coerced is a unary function result, because unary
   functions never return an object or a symbol.
* When the coerced value is a commonly-used numeric identifier:
  `i`, `idx`, or `lineNumber`.
* When the statement immeidately before the coercion is a DEV-only
  call to a function from shared/CheckStringCoercion.js. This call is a
  no-op in production, but in DEV it will show a console error
  explaining the problem, then will throw right after a long explanatory
  code comment so that debugger users will have an idea what's going on.
  The check function call must be in the following format:
    if (__DEV__) {
      checkXxxxxStringCoercion(value);
    };

Manually disabling the rule is usually not necessary because almost all
prod use of the `'' + value` pattern falls into one of the categories
above. But in the rare cases where the rule isn't smart enough to detect
safe usage (e.g. when a coercion is inside a nested ternary operator),
manually disabling the rule will be needed.

The rule should also be manually disabled in prod error handling code
where `String(value)` should be used for coercions, because it'd be
bad to throw while building an error message or stack trace!

The prod and non-prod modes have differentiated error messages to
explain how to do a proper coercion in that mode.

If a production check call is needed but is missing or incorrect
(e.g. not in a DEV block or not immediately before the coercion), then
a context-sensitive error message will be reported so that developers
can figure out what's wrong and how to fix the problem.

Because string coercions are now handled by the `safe-string-coercion`
rule, the `no-primitive-constructor` rule no longer flags `String()`
usage. It still flags `new String(value)` because that usage is almost
always a bug.

* Add DEV-only string coercion check functions

This commit adds DEV-only functions to check whether coercing
values to strings using the `'' + value` pattern will throw. If it will
throw, these functions will:
1. Display a console error with a friendly error message describing
   the problem and the developer can fix it.
2. Perform the coercion, which will throw. Right before the line where
   the throwing happens, there's a long code comment that will help
   debugger users (or others looking at the exception call stack) figure
   out what happened and how to fix the problem.

One of these check functions should be called before all string coercion
of user-provided values, except when the the coercion is guaranteed not
to throw, e.g.
* if inside a typeof check like `if (typeof value === 'string')`
* if coercing the result of a unary function like `+value` or `value++`
* if coercing a variable named in a whitelist of numeric identifiers:
  `i`, `idx`, or `lineNumber`.

The new `safe-string-coercion` internal ESLint rule enforces that
these check functions are called when they are required.

Only use these check functions in production code that will be bundled
with user apps.  For non-prod code (and for production error-handling
code), use `String(value)` instead which may be a little slower but will
never throw.

* Add failing tests for string coercion

Added failing tests to verify:
* That input, select, and textarea elements with value and defaultValue
  set to Temporal-like objects which will throw when coerced to string
  using the `'' + value` pattern.
* That text elements will throw for Temporal-like objects
* That dangerouslySetInnerHTML will *not* throw for Temporal-like
  objects because this value is not cast to a string before passing to
  the DOM.
* That keys that are Temporal-like objects will throw

All tests above validate the friendly error messages thrown.

* Use `String(value)` for coercion in non-prod files

This commit switches non-production code from `'' + value` (which
throws for Temporal objects and symbols) to instead use `String(value)`
which won't throw for these or other future plus-phobic types.

"Non-produciton code" includes anything not bundled into user apps:
* Tests and test utilities. Note that I didn't change legacy React
  test fixtures because I assumed it was good for those files to
  act just like old React, including coercion behavior.
* Build scripts
* Dev tools package - In addition to switching to `String`, I also
  removed special-case code for coercing symbols which is now
  unnecessary.

* Add DEV-only string coercion checks to prod files

This commit adds DEV-only function calls to to check if string coercion
using `'' + value` will throw, which it will if the value is a Temporal
object or a symbol because those types can't be added with `+`.

If it will throw, then in DEV these checks will show a console error
to help the user undertsand what went wrong and how to fix the
problem. After emitting the console error, the check functions will
retry the coercion which will throw with a call stack that's easy (or
at least easier!) to troubleshoot because the exception happens right
after a long comment explaining the issue. So whether the user is in
a debugger, looking at the browser console, or viewing the in-browser
DEV call stack, it should be easy to understand and fix the problem.

In most cases, the safe-string-coercion ESLint rule is smart enough to
detect when a coercion is safe. But in rare cases (e.g. when a coercion
is inside a ternary) this rule will have to be manually disabled.

This commit also switches error-handling code to use `String(value)`
for coercion, because it's bad to crash when you're trying to build
an error message or a call stack!  Because `String()` is usually
disallowed by the `safe-string-coercion` ESLint rule in production
code, the rule must be disabled when `String()` is used.
This commit is contained in:
Justin Grant 2021-09-27 10:05:07 -07:00 committed by GitHub
parent 05726d72cc
commit c88fb49d37
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
58 changed files with 1561 additions and 85 deletions

View File

@ -108,6 +108,10 @@ module.exports = {
// CUSTOM RULES
// the second argument of warning/invariant should be a literal string
'react-internal/no-primitive-constructors': ERROR,
'react-internal/safe-string-coercion': [
ERROR,
{isProductionUserAppCode: true},
],
'react-internal/no-to-warn-dev-within-to-throw': ERROR,
'react-internal/invariant-args': ERROR,
'react-internal/warning-args': ERROR,
@ -168,10 +172,17 @@ module.exports = {
'packages/*/npm/**/*.js',
'packages/dom-event-testing-library/**/*.js',
'packages/react-devtools*/**/*.js',
'dangerfile.js',
'fixtures',
'packages/react-dom/src/test-utils/*.js',
],
rules: {
'react-internal/no-production-logging': OFF,
'react-internal/warning-args': OFF,
'react-internal/safe-string-coercion': [
ERROR,
{isProductionUserAppCode: false},
],
// Disable accessibility checks
'jsx-a11y/aria-role': OFF,
@ -185,7 +196,7 @@ module.exports = {
{
files: [
'scripts/eslint-rules/*.js',
'packages/eslint-plugin-react-hooks/src/*.js'
'packages/eslint-plugin-react-hooks/src/*.js',
],
plugins: ['eslint-plugin'],
rules: {

View File

@ -102,8 +102,8 @@ function row(result) {
let headSha;
let baseSha;
try {
headSha = (readFileSync(HEAD_DIR + '/COMMIT_SHA') + '').trim();
baseSha = (readFileSync(BASE_DIR + '/COMMIT_SHA') + '').trim();
headSha = String(readFileSync(HEAD_DIR + '/COMMIT_SHA')).trim();
baseSha = String(readFileSync(BASE_DIR + '/COMMIT_SHA')).trim();
} catch {
warn(
"Failed to read build artifacts. It's possible a build configuration " +

View File

@ -41,7 +41,7 @@ class ErrorBoundary extends React.Component {
if (this.state.error) {
return <p>Captured an error: {this.state.error.message}</p>;
} else {
return <p>Captured an error: {'' + this.state.error}</p>;
return <p>Captured an error: {String(this.state.error)}</p>;
}
}
if (this.state.shouldThrow) {

View File

@ -615,7 +615,7 @@ describe('ReactHooksInspectionIntegration', () => {
expect(tree[0].id).toEqual(0);
expect(tree[0].isStateEditable).toEqual(false);
expect(tree[0].name).toEqual('OpaqueIdentifier');
expect((tree[0].value + '').startsWith('c_')).toBe(true);
expect(String(tree[0].value).startsWith('c_')).toBe(true);
expect(tree[1]).toEqual({
id: 1,
@ -646,7 +646,7 @@ describe('ReactHooksInspectionIntegration', () => {
expect(tree[0].id).toEqual(0);
expect(tree[0].isStateEditable).toEqual(false);
expect(tree[0].name).toEqual('OpaqueIdentifier');
expect((tree[0].value + '').startsWith('c_')).toBe(true);
expect(String(tree[0].value).startsWith('c_')).toBe(true);
expect(tree[1]).toEqual({
id: 1,

View File

@ -94,7 +94,7 @@ function createPanelIfReactLoaded() {
function initBridgeAndStore() {
const port = chrome.runtime.connect({
name: '' + tabId,
name: String(tabId),
});
// Looks like `port.onDisconnect` does not trigger on in-tab navigation like new URL or back/forward navigation,
// so it makes no sense to handle it here.

View File

@ -63,7 +63,7 @@ function getData(internalInstance: InternalInstance) {
// != used deliberately here to catch undefined and null
if (internalInstance._currentElement != null) {
if (internalInstance._currentElement.key) {
key = '' + internalInstance._currentElement.key;
key = String(internalInstance._currentElement.key);
}
const elementType = internalInstance._currentElement.type;

View File

@ -1848,7 +1848,7 @@ export function attach(
// This check is a guard to handle a React element that has been modified
// in such a way as to bypass the default stringification of the "key" property.
const keyString = key === null ? null : '' + key;
const keyString = key === null ? null : String(key);
const keyStringID = getStringID(keyString);
pushOperation(TREE_OPERATION_ADD);

View File

@ -166,11 +166,7 @@ export function format(
): string {
const args = inputArgs.slice();
// Symbols cannot be concatenated with Strings.
let formatted: string =
typeof maybeMessage === 'symbol'
? maybeMessage.toString()
: '' + maybeMessage;
let formatted: string = String(maybeMessage);
// If the first argument is a string, check for substitutions.
if (typeof maybeMessage === 'string') {
@ -203,17 +199,14 @@ export function format(
// Arguments that remain after formatting.
if (args.length) {
for (let i = 0; i < args.length; i++) {
const arg = args[i];
// Symbols cannot be concatenated with Strings.
formatted += ' ' + (typeof arg === 'symbol' ? arg.toString() : arg);
formatted += ' ' + String(args[i]);
}
}
// Update escaped %% values.
formatted = formatted.replace(/%{2,2}/g, '%');
return '' + formatted;
return String(formatted);
}
export function isSynchronousXHRSupported(): boolean {

View File

@ -46,7 +46,7 @@ export default class ErrorBoundary extends Component<Props, State> {
error !== null &&
error.hasOwnProperty('message')
? error.message
: '' + error;
: String(error);
const callStack =
typeof error === 'object' &&

View File

@ -19,8 +19,8 @@ export function alphaSortEntries(
): number {
const a = entryA[0];
const b = entryB[0];
if ('' + +a === a) {
if ('' + +b !== b) {
if (String(+a) === a) {
if (String(+b) !== b) {
return -1;
}
return +a < +b ? -1 : 1;

View File

@ -180,10 +180,7 @@ export function installHook(target: any): DevToolsHook | null {
const args = inputArgs.slice();
// Symbols cannot be concatenated with Strings.
let formatted: string =
typeof maybeMessage === 'symbol'
? maybeMessage.toString()
: '' + maybeMessage;
let formatted = String(maybeMessage);
// If the first argument is a string, check for substitutions.
if (typeof maybeMessage === 'string') {
@ -216,17 +213,14 @@ export function installHook(target: any): DevToolsHook | null {
// Arguments that remain after formatting.
if (args.length) {
for (let i = 0; i < args.length; i++) {
const arg = args[i];
// Symbols cannot be concatenated with Strings.
formatted += ' ' + (typeof arg === 'symbol' ? arg.toString() : arg);
formatted += ' ' + String(args[i]);
}
}
// Update escaped %% values.
formatted = formatted.replace(/%{2,2}/g, '%');
return '' + formatted;
return String(formatted);
}
let unpatchFn = null;

View File

@ -38,7 +38,6 @@ function normalizeSourcePath(
const {sourceRoot} = map;
let source = sourceInput;
// eslint-disable-next-line react-internal/no-primitive-constructors
source = String(source);
// Some source maps produce relative source paths like "./foo.js" instead of
// "foo.js". Normalize these first so that future comparisons will succeed.

View File

@ -834,7 +834,7 @@ export function formatDataForPreview(
return data;
default:
try {
return truncateForDisplay('' + data);
return truncateForDisplay(String(data));
} catch (error) {
return 'unserializable';
}

View File

@ -89,6 +89,27 @@ describe('ReactDOM unknown attribute', () => {
testUnknownAttributeAssignment(lol, 'lol');
});
it('throws with Temporal-like objects', () => {
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
const test = () =>
testUnknownAttributeAssignment(new TemporalLike(), null);
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Warning: The provided `unknown` attribute is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});
it('removes symbols and warns', () => {
expect(() => testUnknownAttributeRemoval(Symbol('foo'))).toErrorDev(
'Warning: Invalid value for prop `unknown` on <div> tag. Either remove it ' +

View File

@ -254,6 +254,28 @@ describe('ReactDOMComponent', () => {
ReactDOM.render(<span style={style} />, div);
});
it('throws with Temporal-like objects as style values', () => {
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
const style = {fontSize: new TemporalLike()};
const div = document.createElement('div');
const test = () => ReactDOM.render(<span style={style} />, div);
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Warning: The provided `fontSize` CSS property is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});
it('should update styles if initially null', () => {
let styles = null;
const container = document.createElement('div');
@ -1130,7 +1152,7 @@ describe('ReactDOMComponent', () => {
describe('createOpenTagMarkup', () => {
function quoteRegexp(str) {
return (str + '').replace(/([.?*+\^$\[\]\\(){}|-])/g, '\\$1');
return String(str).replace(/([.?*+\^$\[\]\\(){}|-])/g, '\\$1');
}
function expectToHaveAttribute(actual, expected) {
@ -1164,7 +1186,7 @@ describe('ReactDOMComponent', () => {
describe('createContentMarkup', () => {
function quoteRegexp(str) {
return (str + '').replace(/([.?*+\^$\[\]\\(){}|-])/g, '\\$1');
return String(str).replace(/([.?*+\^$\[\]\\(){}|-])/g, '\\$1');
}
function genMarkup(props) {
@ -2412,6 +2434,28 @@ describe('ReactDOMComponent', () => {
expect(el.getAttribute('whatever')).toBe('[object Object]');
});
it('allows Temporal-like objects as HTML (they are not coerced to strings first)', function() {
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
// `dangerouslySetInnerHTML` is never coerced to a string, so won't throw
// even with a Temporal-like object.
const container = document.createElement('div');
ReactDOM.render(
<div dangerouslySetInnerHTML={{__html: new TemporalLike()}} />,
container,
);
expect(container.firstChild.innerHTML).toEqual('2020-01-01');
});
it('allows cased data attributes', function() {
let el;
expect(() => {

View File

@ -544,6 +544,102 @@ describe('ReactDOMInput', () => {
expect(node.value).toBe('foobar');
});
it('should throw for date inputs if `defaultValue` is an object where valueOf() throws', () => {
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
const test = () =>
ReactDOM.render(
<input defaultValue={new TemporalLike()} type="date" />,
container,
);
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.',
);
});
it('should throw for text inputs if `defaultValue` is an object where valueOf() throws', () => {
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
const test = () =>
ReactDOM.render(
<input defaultValue={new TemporalLike()} type="text" />,
container,
);
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.',
);
});
it('should throw for date inputs if `value` is an object where valueOf() throws', () => {
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
const test = () =>
ReactDOM.render(
<input value={new TemporalLike()} type="date" onChange={() => {}} />,
container,
);
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.',
);
});
it('should throw for text inputs if `value` is an object where valueOf() throws', () => {
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
const test = () =>
ReactDOM.render(
<input value={new TemporalLike()} type="text" onChange={() => {}} />,
container,
);
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.',
);
});
it('should display `value` of number 0', () => {
const stub = <input type="text" value={0} onChange={emptyFunction} />;
const node = ReactDOM.render(stub, container);
@ -1575,7 +1671,7 @@ describe('ReactDOMInput', () => {
return value;
},
set: function(val) {
value = '' + val;
value = String(val);
log.push('set property value');
},
});

View File

@ -1019,4 +1019,268 @@ describe('ReactDOMSelect', () => {
expect(node.value).toBe('');
});
});
describe('When given a Temporal.PlainDate-like value', () => {
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
it('throws when given a Temporal.PlainDate-like value (select)', () => {
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} value={new TemporalLike()}>
<option value="2020-01-01">like a Temporal.PlainDate</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props)' +
' must be strings, not TemporalLike. ' +
'This value must be coerced to a string before before using it here.',
);
});
it('throws when given a Temporal.PlainDate-like value (option)', () => {
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} value="2020-01-01">
<option value={new TemporalLike()}>
like a Temporal.PlainDate
</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'The provided `value` attribute is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});
it('throws when given a Temporal.PlainDate-like value (both)', () => {
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} value={new TemporalLike()}>
<option value={new TemporalLike()}>
like a Temporal.PlainDate
</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'The provided `value` attribute is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});
it('throws with updated Temporal.PlainDate-like value (select)', () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} value="monkey">
<option value="2020-01-01">like a Temporal.PlainDate</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} value={new TemporalLike()}>
<option value="2020-01-01">like a Temporal.PlainDate</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props)' +
' must be strings, not TemporalLike. ' +
'This value must be coerced to a string before before using it here.',
);
});
it('throws with updated Temporal.PlainDate-like value (option)', () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} value="2020-01-01">
<option value="donkey">like a Temporal.PlainDate</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} value="2020-01-01">
<option value={new TemporalLike()}>
like a Temporal.PlainDate
</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'The provided `value` attribute is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});
it('throws with updated Temporal.PlainDate-like value (both)', () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} value="donkey">
<option value="donkey">like a Temporal.PlainDate</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} value={new TemporalLike()}>
<option value={new TemporalLike()}>
like a Temporal.PlainDate
</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'The provided `value` attribute is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});
it('throws when given a Temporal.PlainDate-like defaultValue (select)', () => {
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} defaultValue={new TemporalLike()}>
<option value="2020-01-01">like a Temporal.PlainDate</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props)' +
' must be strings, not TemporalLike. ' +
'This value must be coerced to a string before before using it here.',
);
});
it('throws when given a Temporal.PlainDate-like defaultValue (option)', () => {
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} defaultValue="2020-01-01">
<option value={new TemporalLike()}>
like a Temporal.PlainDate
</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'The provided `value` attribute is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});
it('throws when given a Temporal.PlainDate-like value (both)', () => {
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} defaultValue={new TemporalLike()}>
<option value={new TemporalLike()}>
like a Temporal.PlainDate
</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'The provided `value` attribute is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});
it('throws with updated Temporal.PlainDate-like defaultValue (select)', () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} defaultValue="monkey">
<option value="2020-01-01">like a Temporal.PlainDate</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} defaultValue={new TemporalLike()}>
<option value="2020-01-01">like a Temporal.PlainDate</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props)' +
' must be strings, not TemporalLike. ' +
'This value must be coerced to a string before before using it here.',
);
});
it('throws with updated Temporal.PlainDate-like defaultValue (both)', () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} defaultValue="monkey">
<option value="donkey">like a Temporal.PlainDate</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
const test = () => {
ReactTestUtils.renderIntoDocument(
<select onChange={noop} value={new TemporalLike()}>
<option value={new TemporalLike()}>
like a Temporal.PlainDate
</option>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
</select>,
);
};
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'The provided `value` attribute is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
);
});
});
});

View File

@ -1745,7 +1745,7 @@ describe('ReactDOMServerHooks', () => {
it('useOpaqueIdentifier warns when there is a hydration error and we are using ID as a string', async () => {
function Child({appId}) {
return <div aria-labelledby={appId + ''} />;
return <div aria-labelledby={String(appId)} />;
}
function App() {
const id = useOpaqueIdentifier();
@ -1769,7 +1769,7 @@ describe('ReactDOMServerHooks', () => {
it('useOpaqueIdentifier warns when there is a hydration error and we are using ID as a string', async () => {
function Child({appId}) {
return <div aria-labelledby={appId + ''} />;
return <div aria-labelledby={String(appId)} />;
}
function App() {
const id = useOpaqueIdentifier();
@ -1793,7 +1793,7 @@ describe('ReactDOMServerHooks', () => {
it('useOpaqueIdentifier warns if you try to use the result as a string in a child component', async () => {
function Child({appId}) {
return <div aria-labelledby={appId + ''} />;
return <div aria-labelledby={String(appId)} />;
}
function App() {
const id = useOpaqueIdentifier();
@ -1817,7 +1817,7 @@ describe('ReactDOMServerHooks', () => {
it('useOpaqueIdentifier warns if you try to use the result as a string', async () => {
function App() {
const id = useOpaqueIdentifier();
return <div aria-labelledby={id + ''} />;
return <div aria-labelledby={String(id)} />;
}
const container = document.createElement('div');
@ -1836,7 +1836,7 @@ describe('ReactDOMServerHooks', () => {
it('useOpaqueIdentifier warns if you try to use the result as a string in a child component wrapped in a Suspense', async () => {
function Child({appId}) {
return <div aria-labelledby={appId + ''} />;
return <div aria-labelledby={String(appId)} />;
}
function App() {
const id = useOpaqueIdentifier();

View File

@ -242,6 +242,11 @@ describe('ReactDOMServerIntegration - Untrusted URLs - disableJavaScriptURLs', (
// consistency but the code structure makes that hard right now.
expectedToStringCalls = 2;
}
if (__DEV__) {
// Checking for string coercion problems results in double the
// toString calls in DEV
expectedToStringCalls *= 2;
}
let toStringCalls = 0;
const firstIsSafe = {

View File

@ -284,4 +284,26 @@ describe('ReactDOMTextComponent', () => {
ReactDOM.render(<div />, el);
expect(el.innerHTML).toBe('<div></div>');
});
it('throws for Temporal-like text nodes', () => {
const el = document.createElement('div');
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
expect(() =>
ReactDOM.render(<div>{new TemporalLike()}</div>, el),
).toThrowError(
new Error(
'Objects are not valid as a React child (found: object with keys {}).' +
' If you meant to render a collection of children, use an array instead.',
),
);
});
});

View File

@ -142,7 +142,7 @@ describe('ReactDOMTextarea', () => {
return value;
},
set: function(val) {
value = '' + val;
value = String(val);
counter++;
},
});
@ -219,6 +219,36 @@ describe('ReactDOMTextarea', () => {
expect(node.value).toEqual('foo');
});
it('should throw when value is set to a Temporal-like object', () => {
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
const container = document.createElement('div');
const stub = <textarea value="giraffe" onChange={emptyFunction} />;
const node = renderTextarea(stub, container);
expect(node.value).toBe('giraffe');
const test = () =>
ReactDOM.render(
<textarea value={new TemporalLike()} onChange={emptyFunction} />,
container,
);
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'Form field values (value, checked, defaultValue, or defaultChecked props) must be ' +
'strings, not TemporalLike. This value must be coerced to a string before before using it here.',
);
});
it('should take updates to `defaultValue` for uncontrolled textarea', () => {
const container = document.createElement('div');

View File

@ -262,4 +262,33 @@ describe('ReactIdentity', () => {
ReactTestUtils.renderIntoDocument(component);
}).not.toThrow();
});
it('should throw if key is a Temporal-like object', () => {
class TemporalLike {
valueOf() {
// Throwing here is the behavior of ECMAScript "Temporal" date/time API.
// See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
throw new TypeError('prod message');
}
toString() {
return '2020-01-01';
}
}
const el = document.createElement('div');
const test = () =>
ReactDOM.render(
<div>
<span key={new TemporalLike()} />
</div>,
el,
);
expect(() =>
expect(test).toThrowError(new TypeError('prod message')),
).toErrorDev(
'The provided key is an unsupported type TemporalLike.' +
' This value must be coerced to a string before before using it here.',
{withoutStack: true},
);
});
});

View File

@ -44,7 +44,7 @@ const expectChildren = function(container, children) {
} else {
expect(textNode != null).toBe(true);
expect(textNode.nodeType).toBe(3);
expect(textNode.data).toBe('' + children);
expect(textNode.data).toBe(String(children));
}
} else {
let mountIndex = 0;
@ -55,7 +55,7 @@ const expectChildren = function(container, children) {
if (typeof child === 'string') {
textNode = outerNode.childNodes[mountIndex];
expect(textNode.nodeType).toBe(3);
expect(textNode.data).toBe('' + child);
expect(textNode.data).toBe(child);
mountIndex++;
} else {
const elementDOMNode = outerNode.childNodes[mountIndex];

View File

@ -20,6 +20,7 @@ import {
disableJavaScriptURLs,
enableTrustedTypesIntegration,
} from 'shared/ReactFeatureFlags';
import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
import {isOpaqueHydratingObject} from './ReactDOMHostConfig';
import type {PropertyInfo} from '../shared/DOMProperty';
@ -40,10 +41,18 @@ export function getValueForProperty(
const {propertyName} = propertyInfo;
return (node: any)[propertyName];
} else {
// This check protects multiple uses of `expected`, which is why the
// react-internal/safe-string-coercion rule is disabled in several spots
// below.
if (__DEV__) {
checkAttributeStringCoercion(expected, name);
}
if (!disableJavaScriptURLs && propertyInfo.sanitizeURL) {
// If we haven't fully disabled javascript: URLs, and if
// the hydration is successful of a javascript: URL, we
// still want to warn on the client.
// eslint-disable-next-line react-internal/safe-string-coercion
sanitizeURL('' + (expected: any));
}
@ -60,6 +69,7 @@ export function getValueForProperty(
if (shouldRemoveAttribute(name, expected, propertyInfo, false)) {
return value;
}
// eslint-disable-next-line react-internal/safe-string-coercion
if (value === '' + (expected: any)) {
return expected;
}
@ -85,6 +95,7 @@ export function getValueForProperty(
if (shouldRemoveAttribute(name, expected, propertyInfo, false)) {
return stringValue === null ? expected : stringValue;
// eslint-disable-next-line react-internal/safe-string-coercion
} else if (stringValue === '' + (expected: any)) {
return expected;
} else {
@ -119,6 +130,9 @@ export function getValueForAttribute(
return expected === undefined ? undefined : null;
}
const value = node.getAttribute(name);
if (__DEV__) {
checkAttributeStringCoercion(expected, name);
}
if (value === '' + (expected: any)) {
return expected;
}
@ -153,6 +167,9 @@ export function setValueForProperty(
if (value === null) {
node.removeAttribute(attributeName);
} else {
if (__DEV__) {
checkAttributeStringCoercion(value, name);
}
node.setAttribute(
attributeName,
enableTrustedTypesIntegration ? (value: any) : '' + (value: any),
@ -191,6 +208,9 @@ export function setValueForProperty(
if (enableTrustedTypesIntegration) {
attributeValue = (value: any);
} else {
if (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
attributeValue = '' + (value: any);
}
if (propertyInfo.sanitizeURL) {

View File

@ -14,6 +14,7 @@ import {
import {canUseDOM} from 'shared/ExecutionEnvironment';
import hasOwnProperty from 'shared/hasOwnProperty';
import {checkHtmlStringCoercion} from 'shared/CheckStringCoercion';
import {
getValueForAttribute,
@ -139,6 +140,9 @@ if (__DEV__) {
const NORMALIZE_NULL_AND_REPLACEMENT_REGEX = /\u0000|\uFFFD/g;
normalizeMarkupForTextOrAttribute = function(markup: mixed): string {
if (__DEV__) {
checkHtmlStringCoercion(markup);
}
const markupString =
typeof markup === 'string' ? markup : '' + (markup: any);
return markupString

View File

@ -18,6 +18,7 @@ import {checkControlledValueProps} from '../shared/ReactControlledValuePropTypes
import {updateValueIfChanged} from './inputValueTracking';
import getActiveElement from './getActiveElement';
import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags';
import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
import type {ToStringValue} from './ToStringValue';
@ -365,6 +366,9 @@ function updateNamedCousins(rootNode, props) {
// the input might not even be in a form. It might not even be in the
// document. Let's just use the local `querySelectorAll` to ensure we don't
// miss anything.
if (__DEV__) {
checkAttributeStringCoercion(name, 'name');
}
const group = queryRoot.querySelectorAll(
'input[name=' + JSON.stringify('' + name) + '][type="radio"]',
);

View File

@ -7,6 +7,8 @@
* @flow
*/
import {checkFormFieldValueStringCoercion} from 'shared/CheckStringCoercion';
export opaque type ToStringValue =
| boolean
| number
@ -19,6 +21,8 @@ export opaque type ToStringValue =
// around this limitation, we use an opaque type that can only be obtained by
// passing the value through getToStringValue first.
export function toString(value: ToStringValue): string {
// The coercion safety check is performed in getToStringValue().
// eslint-disable-next-line react-internal/safe-string-coercion
return '' + (value: any);
}
@ -26,10 +30,14 @@ export function getToStringValue(value: mixed): ToStringValue {
switch (typeof value) {
case 'boolean':
case 'number':
case 'object':
case 'string':
case 'undefined':
return value;
case 'object':
if (__DEV__) {
checkFormFieldValueStringCoercion(value);
}
return value;
default:
// function, symbol are assigned as empty strings
return '';

View File

@ -7,6 +7,8 @@
* @flow
*/
import {checkFormFieldValueStringCoercion} from 'shared/CheckStringCoercion';
type ValueTracker = {|
getValue(): string,
setValue(value: string): void,
@ -55,6 +57,9 @@ function trackValueOnNode(node: any): ?ValueTracker {
valueField,
);
if (__DEV__) {
checkFormFieldValueStringCoercion(node[valueField]);
}
let currentValue = '' + node[valueField];
// if someone has already defined a value or Safari, then bail
@ -76,6 +81,9 @@ function trackValueOnNode(node: any): ?ValueTracker {
return get.call(this);
},
set: function(value) {
if (__DEV__) {
checkFormFieldValueStringCoercion(value);
}
currentValue = '' + value;
set.call(this, value);
},
@ -93,6 +101,9 @@ function trackValueOnNode(node: any): ?ValueTracker {
return currentValue;
},
setValue(value) {
if (__DEV__) {
checkFormFieldValueStringCoercion(value);
}
currentValue = '' + value;
},
stopTracking() {

View File

@ -16,6 +16,7 @@ import {
shouldRemoveAttribute,
} from '../shared/DOMProperty';
import sanitizeURL from '../shared/sanitizeURL';
import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
import quoteAttributeValueForBrowser from './quoteAttributeValueForBrowser';
/**
@ -44,6 +45,9 @@ export function createMarkupForProperty(name: string, value: mixed): string {
return attributeName + '=""';
} else {
if (propertyInfo.sanitizeURL) {
if (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
value = '' + (value: any);
sanitizeURL(value);
}

View File

@ -9,6 +9,12 @@
import type {ReactNodeList} from 'shared/ReactTypes';
import {
checkHtmlStringCoercion,
checkCSSPropertyStringCoercion,
checkAttributeStringCoercion,
} from 'shared/CheckStringCoercion';
import {Children} from 'react';
import {enableFilterEmptyStringAttributesDOM} from 'shared/ReactFeatureFlags';
@ -272,6 +278,9 @@ function pushStyle(
const isCustomProperty = styleName.indexOf('--') === 0;
if (isCustomProperty) {
nameChunk = stringToChunk(escapeTextForBrowser(styleName));
if (__DEV__) {
checkCSSPropertyStringCoercion(styleValue, styleName);
}
valueChunk = stringToChunk(
escapeTextForBrowser(('' + styleValue).trim()),
);
@ -291,6 +300,9 @@ function pushStyle(
valueChunk = stringToChunk('' + styleValue);
}
} else {
if (__DEV__) {
checkCSSPropertyStringCoercion(styleValue, styleName);
}
valueChunk = stringToChunk(
escapeTextForBrowser(('' + styleValue).trim()),
);
@ -439,6 +451,9 @@ function pushAttribute(
break;
default:
if (propertyInfo.sanitizeURL) {
if (__DEV__) {
checkAttributeStringCoercion(value, attributeName);
}
value = '' + (value: any);
sanitizeURL(value);
}
@ -496,6 +511,9 @@ function pushInnerHTML(
);
const html = innerHTML.__html;
if (html !== null && html !== undefined) {
if (__DEV__) {
checkHtmlStringCoercion(html);
}
target.push(stringToChunk('' + html));
}
}
@ -679,6 +697,9 @@ function pushStartOption(
if (selectedValue !== null) {
let stringValue;
if (value !== null) {
if (__DEV__) {
checkAttributeStringCoercion(value, 'value');
}
stringValue = '' + value;
} else {
if (__DEV__) {
@ -697,6 +718,9 @@ function pushStartOption(
if (isArray(selectedValue)) {
// multiple
for (let i = 0; i < selectedValue.length; i++) {
if (__DEV__) {
checkAttributeStringCoercion(selectedValue[i], 'value');
}
const v = '' + selectedValue[i];
if (v === stringValue) {
target.push(selectedMarkerAttribute);
@ -895,8 +919,16 @@ function pushStartTextArea(
children.length <= 1,
'<textarea> can only have at most one child.',
);
// TODO: remove the coercion and the DEV check below because it will
// always be overwritten by the coercion several lines below it. #22309
if (__DEV__) {
checkHtmlStringCoercion(children[0]);
}
value = '' + children[0];
}
if (__DEV__) {
checkHtmlStringCoercion(children);
}
value = '' + children;
}
@ -1142,6 +1174,9 @@ function pushStartPreformattedElement(
if (typeof html === 'string' && html.length > 0 && html[0] === '\n') {
target.push(leadingNewline, stringToChunk(html));
} else {
if (__DEV__) {
checkHtmlStringCoercion(html);
}
target.push(stringToChunk('' + html));
}
}

View File

@ -25,6 +25,10 @@ import {
enableSuspenseServerRenderer,
enableScopeAPI,
} from 'shared/ReactFeatureFlags';
import {
checkPropStringCoercion,
checkFormFieldValueStringCoercion,
} from 'shared/CheckStringCoercion';
import {
REACT_DEBUG_TRACING_MODE_TYPE,
@ -1472,6 +1476,9 @@ class ReactDOMServerRenderer {
textareaChildren = textareaChildren[0];
}
if (__DEV__) {
checkPropStringCoercion(textareaChildren, 'children');
}
defaultValue = '' + textareaChildren;
}
if (defaultValue == null) {
@ -1480,6 +1487,9 @@ class ReactDOMServerRenderer {
initialValue = defaultValue;
}
if (__DEV__) {
checkFormFieldValueStringCoercion(initialValue);
}
props = Object.assign({}, props, {
value: undefined,
children: '' + initialValue,
@ -1535,6 +1545,9 @@ class ReactDOMServerRenderer {
if (selectValue != null) {
let value;
if (props.value != null) {
if (__DEV__) {
checkFormFieldValueStringCoercion(props.value);
}
value = props.value + '';
} else {
if (__DEV__) {
@ -1554,12 +1567,18 @@ class ReactDOMServerRenderer {
if (isArray(selectValue)) {
// multiple
for (let j = 0; j < selectValue.length; j++) {
if (__DEV__) {
checkFormFieldValueStringCoercion(selectValue[j]);
}
if ('' + selectValue[j] === value) {
selected = true;
break;
}
}
} else {
if (__DEV__) {
checkFormFieldValueStringCoercion(selectValue);
}
selected = '' + selectValue === value;
}

View File

@ -36,6 +36,8 @@
* @private
*/
import {checkHtmlStringCoercion} from 'shared/CheckStringCoercion';
const matchHtmlRegExp = /["'&<>]/;
/**
@ -47,6 +49,9 @@ const matchHtmlRegExp = /["'&<>]/;
*/
function escapeHtml(string) {
if (__DEV__) {
checkHtmlStringCoercion(string);
}
const str = '' + string;
const match = matchHtmlRegExp.exec(str);

View File

@ -6,6 +6,7 @@
*/
import {isUnitlessNumber} from './CSSProperty';
import {checkCSSPropertyStringCoercion} from 'shared/CheckStringCoercion';
/**
* Convert a value into the proper css writable value. The style name `name`
@ -41,6 +42,9 @@ function dangerousStyleValue(name, value, isCustomProperty) {
return value + 'px'; // Presumes implicit 'px' suffix for unitless numbers
}
if (__DEV__) {
checkCSSPropertyStringCoercion(value, name);
}
return ('' + value).trim();
}

View File

@ -95,7 +95,7 @@ function validateClassInstance(inst, methodName) {
return;
}
let received;
const stringified = '' + inst;
const stringified = String(inst);
if (isArray(inst)) {
received = 'an array';
} else if (inst && inst.nodeType === ELEMENT_NODE && inst.tagName) {

View File

@ -22,6 +22,7 @@ import type {RootTag} from 'react-reconciler/src/ReactRootTags';
import * as Scheduler from 'scheduler/unstable_mock';
import {REACT_FRAGMENT_TYPE, REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';
import isArray from 'shared/isArray';
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
import {
DefaultEventPriority,
IdleEventPriority,
@ -215,6 +216,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
keepChildren: boolean,
recyclableInstance: null | Instance,
): Instance {
if (__DEV__) {
checkPropStringCoercion(newProps.children, 'children');
}
const clone = {
id: instance.id,
type: type,
@ -293,13 +297,21 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
if (type === 'errorInCompletePhase') {
throw new Error('Error in host config.');
}
if (__DEV__) {
// The `if` statement here prevents auto-disabling of the safe coercion
// ESLint rule, so we must manually disable it below.
if (shouldSetTextContent(type, props)) {
checkPropStringCoercion(props.children, 'children');
}
}
const inst = {
id: instanceCounter++,
type: type,
children: [],
parent: -1,
text: shouldSetTextContent(type, props)
? computeText((props.children: any) + '', hostContext)
? // eslint-disable-next-line react-internal/safe-string-coercion
computeText((props.children: any) + '', hostContext)
: null,
prop: props.prop,
hidden: !!props.hidden,
@ -481,6 +493,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
instance.prop = newProps.prop;
instance.hidden = !!newProps.hidden;
if (shouldSetTextContent(type, newProps)) {
if (__DEV__) {
checkPropStringCoercion(newProps.children, 'children');
}
instance.text = computeText(
(newProps.children: any) + '',
instance.context,

View File

@ -28,6 +28,7 @@ import {
warnAboutStringRefs,
enableLazyElements,
} from 'shared/ReactFeatureFlags';
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
import {
createWorkInProgress,
@ -163,6 +164,9 @@ function coerceRef(
'bug in React. Please file an issue.',
mixedRef,
);
if (__DEV__) {
checkPropStringCoercion(mixedRef, 'ref');
}
const stringRef = '' + mixedRef;
// Check if previous string ref matches new string ref
if (

View File

@ -28,6 +28,7 @@ import {
warnAboutStringRefs,
enableLazyElements,
} from 'shared/ReactFeatureFlags';
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
import {
createWorkInProgress,
@ -163,6 +164,9 @@ function coerceRef(
'bug in React. Please file an issue.',
mixedRef,
);
if (__DEV__) {
checkPropStringCoercion(mixedRef, 'ref');
}
const stringRef = '' + mixedRef;
// Check if previous string ref matches new string ref
if (

View File

@ -8,6 +8,7 @@
*/
import {REACT_PORTAL_TYPE} from 'shared/ReactSymbols';
import {checkKeyStringCoercion} from 'shared/CheckStringCoercion';
import type {ReactNodeList, ReactPortal} from 'shared/ReactTypes';
@ -18,6 +19,9 @@ export function createPortal(
implementation: any,
key: ?string = null,
): ReactPortal {
if (__DEV__) {
checkKeyStringCoercion(key);
}
return {
// This tag allow us to uniquely identify this as a React Portal
$$typeof: REACT_PORTAL_TYPE,

View File

@ -875,7 +875,7 @@ describe('ReactCache', () => {
function Unrelated() {
const [count, _updateUnrelated] = useState(0);
updateUnrelated = _updateUnrelated;
return <Text text={count + ''} />;
return <Text text={String(count)} />;
}
const root = ReactNoop.createRoot();

View File

@ -2754,7 +2754,7 @@ describe('ReactHooksWithNoopRenderer', () => {
Scheduler.unstable_yieldValue(
`Create insertion [current: ${committedText}]`,
);
committedText = props.count + '';
committedText = String(props.count);
return () => {
Scheduler.unstable_yieldValue(
`Destroy insertion [current: ${committedText}]`,
@ -2817,7 +2817,7 @@ describe('ReactHooksWithNoopRenderer', () => {
Scheduler.unstable_yieldValue(
`Create insertion [current: ${committedText}]`,
);
committedText = props.count + '';
committedText = String(props.count);
return () => {
Scheduler.unstable_yieldValue(
`Destroy insertion [current: ${committedText}]`,
@ -2828,7 +2828,7 @@ describe('ReactHooksWithNoopRenderer', () => {
Scheduler.unstable_yieldValue(
`Create layout [current: ${committedText}]`,
);
committedText = props.count + '';
committedText = String(props.count);
return () => {
Scheduler.unstable_yieldValue(
`Destroy layout [current: ${committedText}]`,
@ -2886,7 +2886,7 @@ describe('ReactHooksWithNoopRenderer', () => {
Scheduler.unstable_yieldValue(
`Create Insertion 1 for Component A [A: ${committedA}, B: ${committedB}]`,
);
committedA = props.count + '';
committedA = String(props.count);
return () => {
Scheduler.unstable_yieldValue(
`Destroy Insertion 1 for Component A [A: ${committedA}, B: ${committedB}]`,
@ -2897,7 +2897,7 @@ describe('ReactHooksWithNoopRenderer', () => {
Scheduler.unstable_yieldValue(
`Create Insertion 2 for Component A [A: ${committedA}, B: ${committedB}]`,
);
committedA = props.count + '';
committedA = String(props.count);
return () => {
Scheduler.unstable_yieldValue(
`Destroy Insertion 2 for Component A [A: ${committedA}, B: ${committedB}]`,
@ -2934,7 +2934,7 @@ describe('ReactHooksWithNoopRenderer', () => {
Scheduler.unstable_yieldValue(
`Create Insertion 1 for Component B [A: ${committedA}, B: ${committedB}]`,
);
committedB = props.count + '';
committedB = String(props.count);
return () => {
Scheduler.unstable_yieldValue(
`Destroy Insertion 1 for Component B [A: ${committedA}, B: ${committedB}]`,
@ -2945,7 +2945,7 @@ describe('ReactHooksWithNoopRenderer', () => {
Scheduler.unstable_yieldValue(
`Create Insertion 2 for Component B [A: ${committedA}, B: ${committedB}]`,
);
committedB = props.count + '';
committedB = String(props.count);
return () => {
Scheduler.unstable_yieldValue(
`Destroy Insertion 2 for Component B [A: ${committedA}, B: ${committedB}]`,
@ -3140,7 +3140,7 @@ describe('ReactHooksWithNoopRenderer', () => {
useLayoutEffect(() => {
// Normally this would go in a mutation effect, but this test
// intentionally omits a mutation effect.
committedText = props.count + '';
committedText = String(props.count);
Scheduler.unstable_yieldValue(
`Mount layout [current: ${committedText}]`,

View File

@ -1110,7 +1110,7 @@ describe('ReactIncremental', () => {
}
render() {
Scheduler.unstable_yieldValue('Bar:' + this.props.x);
return <span prop={'' + (this.props.x === this.state.y)} />;
return <span prop={String(this.props.x === this.state.y)} />;
}
}
@ -1159,7 +1159,7 @@ describe('ReactIncremental', () => {
Scheduler.unstable_yieldValue(
'Bar:' + this.props.x + '-' + this.props.step,
);
return <span prop={'' + (this.props.x === this.state.y)} />;
return <span prop={String(this.props.x === this.state.y)} />;
}
}

View File

@ -223,7 +223,7 @@ describe('memo', () => {
class CounterInner extends React.Component {
static defaultProps = {suffix: '!'};
render() {
return <Text text={this.props.count + '' + this.props.suffix} />;
return <Text text={this.props.count + String(this.props.suffix)} />;
}
}
const Counter = memo(CounterInner);

View File

@ -27,7 +27,7 @@ describe('ReactPersistent', () => {
function createPortal(children, containerInfo, implementation, key) {
return {
$$typeof: Symbol.for('react.portal'),
key: key == null ? null : '' + key,
key: key == null ? null : String(key),
children,
containerInfo,
implementation,

View File

@ -296,7 +296,7 @@ function describeValueForErrorMessage(value: ReactModel): string {
case 'function':
return 'function';
default:
// eslint-disable-next-line
// eslint-disable-next-line react-internal/safe-string-coercion
return String(value);
}
}
@ -615,8 +615,10 @@ function emitErrorChunk(request: Request, id: number, error: mixed): void {
let stack = '';
try {
if (error instanceof Error) {
message = '' + error.message;
stack = '' + error.stack;
// eslint-disable-next-line react-internal/safe-string-coercion
message = String(error.message);
// eslint-disable-next-line react-internal/safe-string-coercion
stack = String(error.stack);
} else {
message = 'Error: ' + (error: any);
}

View File

@ -44,6 +44,7 @@ import invariant from 'shared/invariant';
import isArray from 'shared/isArray';
import getComponentNameFromType from 'shared/getComponentNameFromType';
import ReactVersion from 'shared/ReactVersion';
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
import {getPublicInstance} from './ReactTestHostConfig';
import {ConcurrentRoot, LegacyRoot} from 'react-reconciler/src/ReactRootTags';
@ -251,6 +252,9 @@ function getChildren(parent: Fiber) {
if (validWrapperTypes.has(node.tag)) {
children.push(wrapFiber(node));
} else if (node.tag === HostText) {
if (__DEV__) {
checkPropStringCoercion(node.memoizedProps, 'memoizedProps');
}
children.push('' + node.memoizedProps);
} else {
descend = true;

View File

@ -16,6 +16,7 @@ import {
REACT_ELEMENT_TYPE,
REACT_PORTAL_TYPE,
} from 'shared/ReactSymbols';
import {checkKeyStringCoercion} from 'shared/CheckStringCoercion';
import {isValidElement, cloneAndReplaceKey} from './ReactElement';
@ -65,6 +66,9 @@ function getElementKey(element: any, index: number): string {
// that we don't block potential future ES APIs.
if (typeof element === 'object' && element !== null && element.key != null) {
// Explicit key
if (__DEV__) {
checkKeyStringCoercion(element.key);
}
return escape('' + element.key);
}
// Implicit key determined by the index in the set
@ -119,6 +123,14 @@ function mapIntoArray(
mapIntoArray(mappedChild, array, escapedChildKey, '', c => c);
} else if (mappedChild != null) {
if (isValidElement(mappedChild)) {
if (__DEV__) {
// The `if` statement here prevents auto-disabling of the safe
// coercion ESLint rule, so we must manually disable it below.
// $FlowFixMe Flow incorrectly thinks React.Portal doesn't have a key
if (mappedChild.key && (!child || child.key !== mappedChild.key)) {
checkKeyStringCoercion(mappedChild.key);
}
}
mappedChild = cloneAndReplaceKey(
mappedChild,
// Keep both the (mapped) and old keys if they differ, just as
@ -127,6 +139,7 @@ function mapIntoArray(
// $FlowFixMe Flow incorrectly thinks React.Portal doesn't have a key
(mappedChild.key && (!child || child.key !== mappedChild.key)
? // $FlowFixMe Flow incorrectly thinks existing element's key can be a number
// eslint-disable-next-line react-internal/safe-string-coercion
escapeUserProvidedKey('' + mappedChild.key) + '/'
: '') +
childKey,
@ -190,7 +203,8 @@ function mapIntoArray(
);
}
} else if (type === 'object') {
const childrenString = '' + (children: any);
// eslint-disable-next-line react-internal/safe-string-coercion
const childrenString = String((children: any));
invariant(
false,
'Objects are not valid as a React child (found: %s). ' +

View File

@ -9,6 +9,7 @@ import getComponentNameFromType from 'shared/getComponentNameFromType';
import invariant from 'shared/invariant';
import {REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';
import hasOwnProperty from 'shared/hasOwnProperty';
import {checkKeyStringCoercion} from 'shared/CheckStringCoercion';
import ReactCurrentOwner from './ReactCurrentOwner';
@ -222,10 +223,16 @@ export function jsx(type, config, maybeKey) {
// <div {...props} key="Hi" />, because we aren't currently able to tell if
// key is explicitly declared to be undefined or not.
if (maybeKey !== undefined) {
if (__DEV__) {
checkKeyStringCoercion(maybeKey);
}
key = '' + maybeKey;
}
if (hasValidKey(config)) {
if (__DEV__) {
checkKeyStringCoercion(config.key);
}
key = '' + config.key;
}
@ -286,10 +293,16 @@ export function jsxDEV(type, config, maybeKey, source, self) {
// <div {...props} key="Hi" />, because we aren't currently able to tell if
// key is explicitly declared to be undefined or not.
if (maybeKey !== undefined) {
if (__DEV__) {
checkKeyStringCoercion(maybeKey);
}
key = '' + maybeKey;
}
if (hasValidKey(config)) {
if (__DEV__) {
checkKeyStringCoercion(config.key);
}
key = '' + config.key;
}
@ -366,6 +379,9 @@ export function createElement(type, config, children) {
}
}
if (hasValidKey(config)) {
if (__DEV__) {
checkKeyStringCoercion(config.key);
}
key = '' + config.key;
}
@ -499,6 +515,9 @@ export function cloneElement(element, config, children) {
owner = ReactCurrentOwner.current;
}
if (hasValidKey(config)) {
if (__DEV__) {
checkKeyStringCoercion(config.key);
}
key = '' + config.key;
}

View File

@ -9,6 +9,7 @@ import getComponentNameFromType from 'shared/getComponentNameFromType';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import hasOwnProperty from 'shared/hasOwnProperty';
import {REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';
import {checkKeyStringCoercion} from 'shared/CheckStringCoercion';
const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
@ -222,10 +223,16 @@ export function jsx(type, config, maybeKey) {
// <div {...props} key="Hi" />, because we aren't currently able to tell if
// key is explicitly declared to be undefined or not.
if (maybeKey !== undefined) {
if (__DEV__) {
checkKeyStringCoercion(maybeKey);
}
key = '' + maybeKey;
}
if (hasValidKey(config)) {
if (__DEV__) {
checkKeyStringCoercion(config.key);
}
key = '' + config.key;
}
@ -287,10 +294,16 @@ export function jsxDEV(type, config, maybeKey, source, self) {
// <div {...props} key="Hi" />, because we aren't currently able to tell if
// key is explicitly declared to be undefined or not.
if (maybeKey !== undefined) {
if (__DEV__) {
checkKeyStringCoercion(maybeKey);
}
key = '' + maybeKey;
}
if (hasValidKey(config)) {
if (__DEV__) {
checkKeyStringCoercion(config.key);
}
key = '' + config.key;
}

View File

@ -0,0 +1,167 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/
import {REACT_OPAQUE_ID_TYPE} from 'shared/ReactSymbols';
/*
* The `'' + value` pattern (used in in perf-sensitive code) throws for Symbol
* and Temporal.* types. See https://github.com/facebook/react/pull/22064.
*
* The functions in this module will throw an easier-to-understand,
* easier-to-debug exception with a clear errors message message explaining the
* problem. (Instead of a confusing exception thrown inside the implementation
* of the `value` object).
*/
// $FlowFixMe only called in DEV, so void return is not possible.
function typeName(value: mixed): string {
if (__DEV__) {
// toStringTag is needed for namespaced types like Temporal.Instant
const hasToStringTag = typeof Symbol === 'function' && Symbol.toStringTag;
const type =
(hasToStringTag && (value: any)[Symbol.toStringTag]) ||
(value: any).constructor.name ||
'Object';
return type;
}
}
// $FlowFixMe only called in DEV, so void return is not possible.
function willCoercionThrow(value: mixed): boolean {
if (__DEV__) {
if (
value !== null &&
typeof value === 'object' &&
value.$$typeof === REACT_OPAQUE_ID_TYPE
) {
// OpaqueID type is expected to throw, so React will handle it. Not sure if
// it's expected that string coercion will throw, but we'll assume it's OK.
// See https://github.com/facebook/react/issues/20127.
return;
}
try {
testStringCoercion(value);
return false;
} catch (e) {
return true;
}
}
}
function testStringCoercion(value: mixed) {
// If you ended up here by following an exception call stack, here's what's
// happened: you supplied an object or symbol value to React (as a prop, key,
// DOM attribute, CSS property, string ref, etc.) and when React tried to
// coerce it to a string using `'' + value`, an exception was thrown.
//
// The most common types that will cause this exception are `Symbol` instances
// and Temporal objects like `Temporal.Instant`. But any object that has a
// `valueOf` or `[Symbol.toPrimitive]` method that throws will also cause this
// exception. (Library authors do this to prevent users from using built-in
// numeric operators like `+` or comparison operators like `>=` because custom
// methods are needed to perform accurate arithmetic or comparison.)
//
// To fix the problem, coerce this object or symbol value to a string before
// passing it to React. The most reliable way is usually `String(value)`.
//
// To find which value is throwing, check the browser or debugger console.
// Before this exception was thrown, there should be `console.error` output
// that shows the type (Symbol, Temporal.PlainDate, etc.) that caused the
// problem and how that type was used: key, atrribute, input value prop, etc.
// In most cases, this console output also shows the component and its
// ancestor components where the exception happened.
//
// eslint-disable-next-line react-internal/safe-string-coercion
return '' + (value: any);
}
export function checkAttributeStringCoercion(
value: mixed,
attributeName: string,
) {
if (__DEV__) {
if (willCoercionThrow(value)) {
console.error(
'The provided `%s` attribute is an unsupported type %s.' +
' This value must be coerced to a string before before using it here.',
attributeName,
typeName(value),
);
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
}
}
}
export function checkKeyStringCoercion(value: mixed) {
if (__DEV__) {
if (willCoercionThrow(value)) {
console.error(
'The provided key is an unsupported type %s.' +
' This value must be coerced to a string before before using it here.',
typeName(value),
);
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
}
}
}
export function checkPropStringCoercion(value: mixed, propName: string) {
if (__DEV__) {
if (willCoercionThrow(value)) {
console.error(
'The provided `%s` prop is an unsupported type %s.' +
' This value must be coerced to a string before before using it here.',
propName,
typeName(value),
);
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
}
}
}
export function checkCSSPropertyStringCoercion(value: mixed, propName: string) {
if (__DEV__) {
if (willCoercionThrow(value)) {
console.error(
'The provided `%s` CSS property is an unsupported type %s.' +
' This value must be coerced to a string before before using it here.',
propName,
typeName(value),
);
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
}
}
}
export function checkHtmlStringCoercion(value: mixed) {
if (__DEV__) {
if (willCoercionThrow(value)) {
console.error(
'The provided HTML markup uses a value of unsupported type %s.' +
' This value must be coerced to a string before before using it here.',
typeName(value),
);
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
}
}
}
export function checkFormFieldValueStringCoercion(value: mixed) {
if (__DEV__) {
if (willCoercionThrow(value)) {
console.error(
'Form field values (value, checked, defaultValue, or defaultChecked props)' +
' must be strings, not %s.' +
' This value must be coerced to a string before before using it here.',
typeName(value),
);
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
}
}
}

View File

@ -47,7 +47,8 @@ function printWarning(level, format, args) {
args = args.concat([stack]);
}
const argsWithFormat = args.map(item => '' + item);
// eslint-disable-next-line react-internal/safe-string-coercion
const argsWithFormat = args.map(item => String(item));
// Careful: RN currently depends on this prefix
argsWithFormat.unshift('Warning: ' + format);
// We intentionally don't use spread (or .apply) directly because it

View File

@ -14,7 +14,7 @@ const {RuleTester} = require('eslint');
const ruleTester = new RuleTester();
ruleTester.run('eslint-rules/no-primitive-constructors', rule, {
valid: ['!!obj', "'' + obj", '+string'],
valid: ['!!obj', '+string'],
invalid: [
{
code: 'Boolean(obj)',
@ -26,13 +26,11 @@ ruleTester.run('eslint-rules/no-primitive-constructors', rule, {
],
},
{
code: 'String(obj)',
code: 'new String(obj)',
errors: [
{
message:
'Do not use the String constructor. ' +
'To cast a value to a string, concat it with the empty string ' +
"(unless it's a symbol, which has different semantics): '' + value",
"Do not use `new String()`. Use String() without new (or '' + value for perf-sensitive code).",
},
],
},

View File

@ -0,0 +1,265 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
*/
'use strict';
const rule = require('../safe-string-coercion');
const {RuleTester} = require('eslint');
const ruleTester = new RuleTester();
const missingDevCheckMessage =
'Missing DEV check before this string coercion.' +
' Check should be in this format:\n' +
' if (__DEV__) {\n' +
' checkXxxxxStringCoercion(value);\n' +
' }';
const prevStatementNotDevCheckMessage =
'The statement before this coercion must be a DEV check in this format:\n' +
' if (__DEV__) {\n' +
' checkXxxxxStringCoercion(value);\n' +
' }';
const message =
"Using `'' + value` or `value + ''` is fast to coerce strings, but may throw." +
' For prod code, add a DEV check from shared/CheckStringCoercion immediately' +
' before this coercion.' +
' For non-prod code and prod error handling, use `String(value)` instead.';
ruleTester.run('eslint-rules/safe-string-coercion', rule, {
valid: [
{
code: 'String(obj)',
options: [{isProductionUserAppCode: false}],
},
'String(obj)',
"'a' + obj",
`
function getValueForAttribute(
node,
name,
expected
) {
if (__DEV__) {
var value = node.getAttribute(name);
if (__DEV__) {
checkAttributeStringCoercion(expected, name);
}
if (value === '' + expected) {
return expected;
}
return value;
}
}
`,
`
if (__DEV__) { checkFormFieldValueStringCoercion (obj) }
'' + obj;
`,
`
function f(a, index) {
if (typeof a === 'object' && a !== null && a.key != null) {
if (__DEV__) {
checkKeyStringCoercion(a.key);
}
return f('' + a.key);
}
return a;
}
`,
"'' + i++",
"'' + +i",
"'' + +i",
"+i + ''",
"if (typeof obj === 'string') { '' + obj }",
"if (typeof obj === 'string' || typeof obj === 'number') { '' + obj }",
"if (typeof obj === 'string' && somethingElse) { '' + obj }",
"if (typeof obj === 'number' && somethingElse) { '' + obj }",
"if (typeof obj === 'bigint' && somethingElse) { '' + obj }",
"if (typeof obj === 'undefined' && somethingElse) { '' + obj }",
"if (typeof nextProp === 'number') { setTextContent(domElement, '' + nextProp); }",
// These twe below are sneaky. The inner `if` is unsafe, but the outer `if`
// ensures that the unsafe code will never be run. It's bad code, but
// doesn't violate this rule.
"if (typeof obj === 'string') { if (typeof obj === 'string' && obj.length) {} else {'' + obj} }",
"if (typeof obj === 'string') if (typeof obj === 'string' && obj.length) {} else {'' + obj}",
],
invalid: [
{
code: "'' + obj",
errors: [
{
message: missingDevCheckMessage + '\n' + message,
},
],
},
{
code: "obj + ''",
errors: [
{
message: missingDevCheckMessage + '\n' + message,
},
],
},
{
code: 'String(obj)',
options: [{isProductionUserAppCode: true}],
errors: [
{
message:
"For perf-sensitive coercion, avoid `String(value)`. Instead, use `'' + value`." +
' Precede it with a DEV check from shared/CheckStringCoercion' +
' unless Symbol and Temporal.* values are impossible.' +
' For non-prod code and prod error handling, use `String(value)` and disable this rule.',
},
],
},
{
code: "if (typeof obj === 'object') { '' + obj }",
errors: [
{
message: missingDevCheckMessage + '\n' + message,
},
],
},
{
code:
"if (typeof obj === 'string') { } else if (typeof obj === 'object') {'' + obj}",
errors: [
{
message: missingDevCheckMessage + '\n' + message,
},
],
},
{
code: "if (typeof obj === 'string' && obj.length) {} else {'' + obj}",
errors: [
{
message: missingDevCheckMessage + '\n' + message,
},
],
},
{
code: `
if (__D__) { checkFormFieldValueStringCoercion (obj) }
'' + obj;
`,
errors: [
{
message: prevStatementNotDevCheckMessage + '\n' + message,
},
],
},
{
code: `
if (__DEV__) { checkFormFieldValueStringCoercion (obj) }
'' + notobjj;
`,
errors: [
{
message:
'Value passed to the check function before this coercion must match the value being coerced.' +
'\n' +
message,
},
],
},
{
code: `
if (__DEV__) { checkFormFieldValueStringCoercion (obj) }
// must be right before the check call
someOtherCode();
'' + objj;
`,
errors: [
{
message: prevStatementNotDevCheckMessage + '\n' + message,
},
],
},
{
code: `
if (__DEV__) { chexxxxBadNameCoercion (obj) }
'' + objj;
`,
errors: [
{
message:
'Missing or invalid check function call before this coercion.' +
' Expected: call of a function like checkXXXStringCoercion. ' +
prevStatementNotDevCheckMessage +
'\n' +
message,
},
],
},
{
code: `
if (__DEV__) { }
'' + objj;
`,
errors: [
{
message: prevStatementNotDevCheckMessage + '\n' + message,
},
],
},
{
code: `
if (__DEV__) { if (x) {} }
'' + objj;
`,
errors: [
{
message:
'The DEV block before this coercion must only contain an expression. ' +
prevStatementNotDevCheckMessage +
'\n' +
message,
},
],
},
{
code: `
if (a) {
if (__DEV__) {
// can't have additional code before the check call
if (b) {
checkKeyStringCoercion(obj);
}
}
g = f( c, d + (b ? '' + obj : '') + e);
}
`,
errors: [
{
message:
'The DEV block before this coercion must only contain an expression. ' +
prevStatementNotDevCheckMessage +
'\n' +
message,
},
],
},
{
code: `
if (__DEV__) {
checkAttributeStringCoercion(expected, name);
}
// DEV check should be inside the if block
if (a && b) {
f('' + expected);
}
`,
errors: [
{
message: missingDevCheckMessage + '\n' + message,
},
],
},
],
});

View File

@ -9,5 +9,6 @@ module.exports = {
'no-production-logging': require('./no-production-logging'),
'no-cross-fork-imports': require('./no-cross-fork-imports'),
'no-cross-fork-types': require('./no-cross-fork-types'),
'safe-string-coercion': require('./safe-string-coercion'),
},
};

View File

@ -29,13 +29,12 @@ module.exports = {
);
break;
case 'String':
report(
node,
name,
'To cast a value to a string, concat it with the empty string ' +
"(unless it's a symbol, which has different semantics): " +
"'' + value"
);
if (node.type === 'NewExpression') {
context.report(
node,
"Do not use `new String()`. Use String() without new (or '' + value for perf-sensitive code)."
);
}
break;
case 'Number':
report(

View File

@ -0,0 +1,344 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
*/
'use strict';
function isEmptyLiteral(node) {
return (
node.type === 'Literal' &&
typeof node.value === 'string' &&
node.value === ''
);
}
// Symbols and Temporal.* objects will throw when using `'' + value`, but that
// pattern can be faster than `String(value)` because JS engines can optimize
// `+` better in some cases. Therefore, in perf-sensitive production codepaths
// we require using `'' + value` for string coercion. The only exception is prod
// error handling code, because it's bad to crash while assembling an error
// message or call stack! Also, error-handling code isn't usually perf-critical.
//
// Non-production codepaths (tests, devtools extension, build tools, etc.)
// should use `String(value)` because it will never crash and the (small) perf
// difference doesn't matter enough for non-prod use cases.
//
// This rule assists enforcing these guidelines:
// * `'' + value` is flagged with a message to remind developers to add a DEV
// check from shared/CheckStringCoercion.js to make sure that the user gets a
// clear error message in DEV is the coercion will throw. These checks are not
// needed if throwing is not possible, e.g. if the value is already known to
// be a string or number.
// * `String(value)` is flagged only if the `isProductionUserAppCode` option
// is set. Set this option for prod code files, and don't set it for non-prod
// files.
const ignoreKeys = [
'range',
'raw',
'parent',
'loc',
'start',
'end',
'_babelType',
'leadingComments',
'trailingComments',
];
function astReplacer(key, value) {
return ignoreKeys.includes(key) ? undefined : value;
}
/**
* Simplistic comparison between AST node. Only the following patterns are
* supported because that's almost all (all?) usage in React:
* - Identifiers, e.g. `foo`
* - Member access, e.g. `foo.bar`
* - Array access with numeric literal, e.g. `foo[0]`
*/
function isEquivalentCode(node1, node2) {
return (
JSON.stringify(node1, astReplacer) === JSON.stringify(node2, astReplacer)
);
}
function isDescendant(node, maybeParentNode) {
let parent = node.parent;
while (parent) {
if (!parent) {
return false;
}
if (parent === maybeParentNode) {
return true;
}
parent = parent.parent;
}
return false;
}
function isSafeTypeofExpression(originalValueNode, node) {
if (node.type === 'BinaryExpression') {
// Example: typeof foo === 'string'
if (node.operator !== '===') {
return false;
}
const {left, right} = node;
// left must be `typeof original`
if (left.type !== 'UnaryExpression' || left.operator !== 'typeof') {
return false;
}
if (!isEquivalentCode(left.argument, originalValueNode)) {
return false;
}
// right must be a literal value of a safe type
const safeTypes = ['string', 'number', 'boolean', 'undefined', 'bigint'];
if (right.type !== 'Literal' || !safeTypes.includes(right.value)) {
return false;
}
return true;
} else if (node.type === 'LogicalExpression') {
// Examples:
// * typeof foo === 'string' && typeof foo === 'number
// * typeof foo === 'string' && someOtherTest
if (node.operator === '&&') {
return (
isSafeTypeofExpression(originalValueNode, node.left) ||
isSafeTypeofExpression(originalValueNode, node.right)
);
} else if (node.operator === '||') {
return (
isSafeTypeofExpression(originalValueNode, node.left) &&
isSafeTypeofExpression(originalValueNode, node.right)
);
}
}
return false;
}
/**
Returns true if the code is inside an `if` block that validates the value
excludes symbols and objects. Examples:
* if (typeof value === 'string') { }
* if (typeof value === 'string' || typeof value === 'number') { }
* if (typeof value === 'string' || someOtherTest) { }
@param - originalValueNode Top-level expression to test. Kept unchanged during
recursion.
@param - node Expression to test at current recursion level. Will be undefined
on non-recursive call.
*/
function isInSafeTypeofBlock(originalValueNode, node) {
if (!node) {
node = originalValueNode;
}
let parent = node.parent;
while (parent) {
if (!parent) {
return false;
}
// Normally, if the parent block is inside a type-safe `if` statement,
// then all child code is also type-safe. But there's a quirky case we
// need to defend against:
// if (typeof obj === 'string') { } else if (typeof obj === 'object') {'' + obj}
// if (typeof obj === 'string') { } else {'' + obj}
// In that code above, the `if` block is safe, but the `else` block is
// unsafe and should report. But the AST parent of the `else` clause is the
// `if` statement. This is the one case where the parent doesn't confer
// safety onto the child. The code below identifies that case and keeps
// moving up the tree until we get out of the `else`'s parent `if` block.
// This ensures that we don't use any of these "parents" (really siblings)
// to confer safety onto the current node.
if (
parent.type === 'IfStatement' &&
!isDescendant(originalValueNode, parent.alternate)
) {
const test = parent.test;
if (isSafeTypeofExpression(originalValueNode, test)) {
return true;
}
}
parent = parent.parent;
}
}
const missingDevCheckMessage =
'Missing DEV check before this string coercion.' +
' Check should be in this format:\n' +
' if (__DEV__) {\n' +
' checkXxxxxStringCoercion(value);\n' +
' }';
const prevStatementNotDevCheckMessage =
'The statement before this coercion must be a DEV check in this format:\n' +
' if (__DEV__) {\n' +
' checkXxxxxStringCoercion(value);\n' +
' }';
/**
* Does this node have an "is coercion safe?" DEV check
* in the same block?
*/
function hasCoercionCheck(node) {
// find the containing statement
let topOfExpression = node;
while (!topOfExpression.parent.body) {
topOfExpression = topOfExpression.parent;
if (!topOfExpression) {
return 'Cannot find top of expression.';
}
}
const containingBlock = topOfExpression.parent.body;
const index = containingBlock.indexOf(topOfExpression);
if (index <= 0) {
return missingDevCheckMessage;
}
const prev = containingBlock[index - 1];
// The previous statement is expected to be like this:
// if (__DEV__) {
// checkFormFieldValueStringCoercion(foo);
// }
// where `foo` must be equivalent to `node` (which is the
// mixed value being coerced to a string).
if (
prev.type !== 'IfStatement' ||
prev.test.type !== 'Identifier' ||
prev.test.name !== '__DEV__'
) {
return prevStatementNotDevCheckMessage;
}
let maybeCheckNode = prev.consequent;
if (maybeCheckNode.type === 'BlockStatement') {
const body = maybeCheckNode.body;
if (body.length === 0) {
return prevStatementNotDevCheckMessage;
}
if (body.length !== 1) {
return (
'Too many statements in DEV block before this coercion.' +
' Expected only one (the check function call). ' +
prevStatementNotDevCheckMessage
);
}
maybeCheckNode = body[0];
}
if (maybeCheckNode.type !== 'ExpressionStatement') {
return (
'The DEV block before this coercion must only contain an expression. ' +
prevStatementNotDevCheckMessage
);
}
const call = maybeCheckNode.expression;
if (
call.type !== 'CallExpression' ||
call.callee.type !== 'Identifier' ||
!/^check(\w+?)StringCoercion$/.test(call.callee.name) ||
!call.arguments.length
) {
// `maybeCheckNode` should be a call of a function named checkXXXStringCoercion
return (
'Missing or invalid check function call before this coercion.' +
' Expected: call of a function like checkXXXStringCoercion. ' +
prevStatementNotDevCheckMessage
);
}
const same = isEquivalentCode(call.arguments[0], node);
if (!same) {
return (
'Value passed to the check function before this coercion' +
' must match the value being coerced.'
);
}
}
function plusEmptyString(context, node) {
if (
node.operator === '+' &&
(isEmptyLiteral(node.left) || isEmptyLiteral(node.right))
) {
let valueToTest = isEmptyLiteral(node.left) ? node.right : node.left;
if (valueToTest.type === 'TypeCastExpression' && valueToTest.expression) {
valueToTest = valueToTest.expression;
}
if (
valueToTest.type === 'Identifier' &&
['i', 'idx', 'lineNumber'].includes(valueToTest.name)
) {
// Common non-object variable names are assumed to be safe
return;
}
if (
valueToTest.type === 'UnaryExpression' ||
valueToTest.type === 'UpdateExpression'
) {
// Any unary expression will return a non-object, non-symbol type.
return;
}
if (isInSafeTypeofBlock(valueToTest)) {
// The value is inside an if (typeof...) block that ensures it's safe
return;
}
const coercionCheckMessage = hasCoercionCheck(valueToTest);
if (!coercionCheckMessage) {
// The previous statement is a correct check function call, so no report.
return;
}
context.report({
node,
message:
coercionCheckMessage +
'\n' +
"Using `'' + value` or `value + ''` is fast to coerce strings, but may throw." +
' For prod code, add a DEV check from shared/CheckStringCoercion immediately' +
' before this coercion.' +
' For non-prod code and prod error handling, use `String(value)` instead.',
});
}
}
function coerceWithStringConstructor(context, node) {
const isProductionUserAppCode =
context.options[0] && context.options[0].isProductionUserAppCode;
if (isProductionUserAppCode && node.callee.name === 'String') {
context.report(
node,
"For perf-sensitive coercion, avoid `String(value)`. Instead, use `'' + value`." +
' Precede it with a DEV check from shared/CheckStringCoercion' +
' unless Symbol and Temporal.* values are impossible.' +
' For non-prod code and prod error handling, use `String(value)` and disable this rule.'
);
}
}
module.exports = {
meta: {
schema: [
{
type: 'object',
properties: {
isProductionUserAppCode: {
type: 'boolean',
default: false,
},
},
additionalProperties: false,
},
],
},
create(context) {
return {
BinaryExpression: node => plusEmptyString(context, node),
CallExpression: node => coerceWithStringConstructor(context, node),
};
},
};

View File

@ -33,8 +33,8 @@ async function runFlow(renderer, args) {
const srcStat = fs.statSync(__dirname + '/config/flowconfig');
const destPath = './.flowconfig';
if (fs.existsSync(destPath)) {
const oldConfig = fs.readFileSync(destPath) + '';
const newConfig = fs.readFileSync(srcPath) + '';
const oldConfig = String(fs.readFileSync(destPath));
const newConfig = String(fs.readFileSync(srcPath));
if (oldConfig !== newConfig) {
// Use the mtime to detect if the file was manually edited. If so,
// log an error.

View File

@ -90,7 +90,7 @@ for (const {base: baseFilename, from, to} of getTransforms()) {
}
);
if (gitShowResult.status !== 0) {
console.error('' + gitShowResult.stderr);
console.error(String(gitShowResult.stderr));
continue;
}
@ -104,6 +104,6 @@ for (const {base: baseFilename, from, to} of getTransforms()) {
});
if (mergeFileResult.status !== 0) {
console.error('' + mergeFileResult.stderr);
console.error(String(mergeFileResult.stderr));
}
}

View File

@ -18,13 +18,13 @@ const {
// Runs the build script for both stable and experimental release channels,
// by configuring an environment variable.
const sha = (
spawnSync('git', ['show', '-s', '--format=%h']).stdout + ''
const sha = String(
spawnSync('git', ['show', '-s', '--format=%h']).stdout
).trim();
let dateString = (
let dateString = String(
spawnSync('git', ['show', '-s', '--format=%cd', '--date=format:%Y%m%d', sha])
.stdout + ''
.stdout
).trim();
// On CI environment, this string is wrapped with quotes '...'s