Skip to content

Commit

Permalink
Replace '' + obj pattern with ''.concat(obj)
Browse files Browse the repository at this point in the history
Using the `+` operator to concatenate strings and objects is problematic
because `+` calls `valueOf` on the object. `valueOf` will throw for
some types of objects. See facebook#20594 for details.

This commit globally replaces `'' + obj` with `''.concat(obj)`, which
does the same thing but doesn't call `valueOf` before converting to a
string.

It also updates the code example in the lint rule that encouaged use of
this problematic pattern.
  • Loading branch information
justingrant committed Aug 16, 2021
1 parent fd7b4a5 commit fa75406
Show file tree
Hide file tree
Showing 55 changed files with 144 additions and 126 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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: {''.concat(this.state.error)}</p>;
}
}
if (this.state.shouldThrow) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ function printWarning(level, format, args) {
}

var argsWithFormat = args.map(function (item) {
return '' + item;
return ''.concat(item);
}); // Careful: RN currently depends on this prefix

argsWithFormat.unshift('Warning: ' + format); // We intentionally don't use spread (or .apply) directly because it
Expand Down Expand Up @@ -483,11 +483,11 @@ function jsxDEV(type, config, maybeKey, source, self) {
// key is explicitly declared to be undefined or not.

if (maybeKey !== undefined) {
key = '' + maybeKey;
key = ''.concat(maybeKey);
}

if (hasValidKey(config)) {
key = '' + config.key;
key = ''.concat(config.key);
}

if (hasValidRef(config)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function printWarning(level, format, args) {
}

var argsWithFormat = args.map(function (item) {
return '' + item;
return ''.concat(item);
}); // Careful: RN currently depends on this prefix

argsWithFormat.unshift('Warning: ' + format); // We intentionally don't use spread (or .apply) directly because it
Expand Down Expand Up @@ -487,11 +487,11 @@ function jsxDEV(type, config, maybeKey, source, self) {
// key is explicitly declared to be undefined or not.

if (maybeKey !== undefined) {
key = '' + maybeKey;
key = ''.concat(maybeKey);
}

if (hasValidKey(config)) {
key = '' + config.key;
key = ''.concat(config.key);
}

if (hasValidRef(config)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ function printWarning(level, format, args) {
}

var argsWithFormat = args.map(function (item) {
return '' + item;
return ''.concat(item);
}); // Careful: RN currently depends on this prefix

argsWithFormat.unshift('Warning: ' + format); // We intentionally don't use spread (or .apply) directly because it
Expand Down Expand Up @@ -488,11 +488,11 @@ function jsxDEV(type, config, maybeKey, source, self) {
// key is explicitly declared to be undefined or not.

if (maybeKey !== undefined) {
key = '' + maybeKey;
key = ''.concat(maybeKey);
}

if (hasValidKey(config)) {
key = '' + config.key;
key = ''.concat(config.key);
}

if (hasValidRef(config)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ function printWarning(level, format, args) {
}

var argsWithFormat = args.map(function (item) {
return '' + item;
return ''.concat(item);
}); // Careful: RN currently depends on this prefix

argsWithFormat.unshift('Warning: ' + format); // We intentionally don't use spread (or .apply) directly because it
Expand Down Expand Up @@ -492,11 +492,11 @@ function jsxDEV(type, config, maybeKey, source, self) {
// key is explicitly declared to be undefined or not.

if (maybeKey !== undefined) {
key = '' + maybeKey;
key = ''.concat(maybeKey);
}

if (hasValidKey(config)) {
key = '' + config.key;
key = ''.concat(config.key);
}

if (hasValidRef(config)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ function printWarning(level, format, args) {
}

var argsWithFormat = args.map(function (item) {
return '' + item;
return ''.concat(item);
}); // Careful: RN currently depends on this prefix

argsWithFormat.unshift('Warning: ' + format); // We intentionally don't use spread (or .apply) directly because it
Expand Down Expand Up @@ -509,11 +509,11 @@ function jsxDEV(type, config, maybeKey, source, self) {
// key is explicitly declared to be undefined or not.

if (maybeKey !== undefined) {
key = '' + maybeKey;
key = ''.concat(maybeKey);
}

if (hasValidKey(config)) {
key = '' + config.key;
key = ''.concat(config.key);
}

if (hasValidRef(config)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ function printWarning(level, format, args) {
}

var argsWithFormat = args.map(function (item) {
return '' + item;
return ''.concat(item);
}); // Careful: RN currently depends on this prefix

argsWithFormat.unshift('Warning: ' + format); // We intentionally don't use spread (or .apply) directly because it
Expand Down Expand Up @@ -513,11 +513,11 @@ function jsxDEV(type, config, maybeKey, source, self) {
// key is explicitly declared to be undefined or not.

if (maybeKey !== undefined) {
key = '' + maybeKey;
key = ''.concat(maybeKey);
}

if (hasValidKey(config)) {
key = '' + config.key;
key = ''.concat(config.key);
}

if (hasValidRef(config)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function printWarning(level, format, args) {
}

var argsWithFormat = args.map(function (item) {
return '' + item;
return ''.concat(item);
}); // Careful: RN currently depends on this prefix

argsWithFormat.unshift('Warning: ' + format); // We intentionally don't use spread (or .apply) directly because it
Expand Down Expand Up @@ -815,11 +815,11 @@ function jsxDEV(type, config, maybeKey, source, self) {
// key is explicitly declared to be undefined or not.

if (maybeKey !== undefined) {
key = '' + maybeKey;
key = ''.concat(maybeKey);
}

if (hasValidKey(config)) {
key = '' + config.key;
key = ''.concat(config.key);
}

if (hasValidRef(config)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function printWarning(level, format, args) {
}

var argsWithFormat = args.map(function (item) {
return '' + item;
return ''.concat(item);
}); // Careful: RN currently depends on this prefix

argsWithFormat.unshift('Warning: ' + format); // We intentionally don't use spread (or .apply) directly because it
Expand Down Expand Up @@ -815,11 +815,11 @@ function jsxDEV(type, config, maybeKey, source, self) {
// key is explicitly declared to be undefined or not.

if (maybeKey !== undefined) {
key = '' + maybeKey;
key = ''.concat(maybeKey);
}

if (hasValidKey(config)) {
key = '' + config.key;
key = ''.concat(config.key);
}

if (hasValidRef(config)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-extensions/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function createPanelIfReactLoaded() {

function initBridgeAndStore() {
const port = chrome.runtime.connect({
name: '' + tabId,
name: ''.concat(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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ''.concat(internalInstance._currentElement.key);
}

const elementType = internalInstance._currentElement.type;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1838,7 +1838,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 : ''.concat(key);
const keyStringID = getStringID(keyString);

pushOperation(TREE_OPERATION_ADD);
Expand Down
7 changes: 4 additions & 3 deletions packages/react-devtools-shared/src/backend/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export function format(
let formatted: string =
typeof maybeMessage === 'symbol'
? maybeMessage.toString()
: '' + maybeMessage;
: ''.concat(maybeMessage);

// If the first argument is a string, check for substitutions.
if (typeof maybeMessage === 'string') {
Expand Down Expand Up @@ -205,14 +205,15 @@ export function format(
const arg = args[i];

// Symbols cannot be concatenated with Strings.
formatted += ' ' + (typeof arg === 'symbol' ? arg.toString() : arg);
formatted +=
' ' + (typeof arg === 'symbol' ? arg.toString() : ''.concat(arg));
}
}

// Update escaped %% values.
formatted = formatted.replace(/%{2,2}/g, '%');

return '' + formatted;
return ''.concat(formatted);
}

export function isSynchronousXHRSupported(): boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export default class ErrorBoundary extends Component<Props, State> {
error !== null &&
error.hasOwnProperty('message')
? error.message
: '' + error;
: ''.concat(error);

const callStack =
typeof error === 'object' &&
Expand Down
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/devtools/views/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ export function alphaSortEntries(
): number {
const a = entryA[0];
const b = entryB[0];
if ('' + +a === a) {
if ('' + +b !== b) {
if (''.concat(+a) === a) {
if (''.concat(+b) !== b) {
return -1;
}
return +a < +b ? -1 : 1;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ export function formatDataForPreview(
return data;
default:
try {
return truncateForDisplay('' + data);
return truncateForDisplay(''.concat(data));
} catch (error) {
return 'unserializable';
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1647,7 +1647,7 @@ describe('ReactDOMInput', () => {
return value;
},
set: function(val) {
value = '' + val;
value = ''.concat(val);
log.push('set property value');
},
});
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/ReactDOMTextarea-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ describe('ReactDOMTextarea', () => {
return value;
},
set: function(val) {
value = '' + val;
value = ''.concat(val);
counter++;
},
});
Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/src/__tests__/ReactMultiChildText-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(''.concat(children));
}
} else {
let mountIndex = 0;
Expand All @@ -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(''.concat(child));
mountIndex++;
} else {
const elementDOMNode = outerNode.childNodes[mountIndex];
Expand Down
16 changes: 9 additions & 7 deletions packages/react-dom/src/client/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function getValueForProperty(
// 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.
sanitizeURL('' + (expected: any));
sanitizeURL(''.concat((expected: any)));
}

const attributeName = propertyInfo.attributeName;
Expand All @@ -60,7 +60,7 @@ export function getValueForProperty(
if (shouldRemoveAttribute(name, expected, propertyInfo, false)) {
return value;
}
if (value === '' + (expected: any)) {
if (value === ''.concat((expected: any))) {
return expected;
}
return value;
Expand All @@ -85,7 +85,7 @@ export function getValueForProperty(

if (shouldRemoveAttribute(name, expected, propertyInfo, false)) {
return stringValue === null ? expected : stringValue;
} else if (stringValue === '' + (expected: any)) {
} else if (stringValue === ''.concat((expected: any))) {
return expected;
} else {
return stringValue;
Expand Down Expand Up @@ -119,7 +119,7 @@ export function getValueForAttribute(
return expected === undefined ? undefined : null;
}
const value = node.getAttribute(name);
if (value === '' + (expected: any)) {
if (value === ''.concat((expected: any))) {
return expected;
}
return value;
Expand Down Expand Up @@ -155,7 +155,9 @@ export function setValueForProperty(
} else {
node.setAttribute(
attributeName,
enableTrustedTypesIntegration ? (value: any) : '' + (value: any),
enableTrustedTypesIntegration
? (value: any)
: ''.concat((value: any)),
);
}
}
Expand Down Expand Up @@ -187,11 +189,11 @@ export function setValueForProperty(
attributeValue = '';
} else {
// `setAttribute` with objects becomes only `[object]` in IE8/9,
// ('' + value) makes it output the correct toString()-value.
// (''.concat(value)) makes it output the correct toString()-value.
if (enableTrustedTypesIntegration) {
attributeValue = (value: any);
} else {
attributeValue = '' + (value: any);
attributeValue = ''.concat((value: any));
}
if (propertyInfo.sanitizeURL) {
sanitizeURL(attributeValue.toString());
Expand Down
Loading

0 comments on commit fa75406

Please sign in to comment.