Skip to content

Commit

Permalink
Change ''.concat(value) to String(value)
Browse files Browse the repository at this point in the history
`''.concat(value)` violates current Flow definitons that require args to
concat to be strings only, even though the MDN docs and ES spec allow
args to be any type. See facebook/flow#8728.

To avoid depending on a change to Flow, this commit changes
`''.concat(value)` to `String(value)` and removes the lint rule
that prohibits use of String(value). This rule was added in #9082.
The other rules in that PR (for Boolean and Number constructors)
are not changed in this commit, only the rule for String.

If it's decided that using `concat` is better, simply omit this commit
and use the previous one instead.
  • Loading branch information
justingrant committed Aug 16, 2021
1 parent fa75406 commit a3c037b
Show file tree
Hide file tree
Showing 55 changed files with 124 additions and 161 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: {''.concat(this.state.error)}</p>;
return <p>Captured an error: {String(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 ''.concat(item);
return String(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 = ''.concat(maybeKey);
key = String(maybeKey);
}

if (hasValidKey(config)) {
key = ''.concat(config.key);
key = String(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 ''.concat(item);
return String(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 = ''.concat(maybeKey);
key = String(maybeKey);
}

if (hasValidKey(config)) {
key = ''.concat(config.key);
key = String(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 ''.concat(item);
return String(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 = ''.concat(maybeKey);
key = String(maybeKey);
}

if (hasValidKey(config)) {
key = ''.concat(config.key);
key = String(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 ''.concat(item);
return String(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 = ''.concat(maybeKey);
key = String(maybeKey);
}

if (hasValidKey(config)) {
key = ''.concat(config.key);
key = String(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 ''.concat(item);
return String(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 = ''.concat(maybeKey);
key = String(maybeKey);
}

if (hasValidKey(config)) {
key = ''.concat(config.key);
key = String(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 ''.concat(item);
return String(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 = ''.concat(maybeKey);
key = String(maybeKey);
}

if (hasValidKey(config)) {
key = ''.concat(config.key);
key = String(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 ''.concat(item);
return String(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 = ''.concat(maybeKey);
key = String(maybeKey);
}

if (hasValidKey(config)) {
key = ''.concat(config.key);
key = String(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 ''.concat(item);
return String(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 = ''.concat(maybeKey);
key = String(maybeKey);
}

if (hasValidKey(config)) {
key = ''.concat(config.key);
key = String(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: ''.concat(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.
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 = ''.concat(internalInstance._currentElement.key);
key = String(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 : ''.concat(key);
const keyString = key === null ? null : String(key);
const keyStringID = getStringID(keyString);

pushOperation(TREE_OPERATION_ADD);
Expand Down
6 changes: 3 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()
: ''.concat(maybeMessage);
: String(maybeMessage);

// If the first argument is a string, check for substitutions.
if (typeof maybeMessage === 'string') {
Expand Down Expand Up @@ -206,14 +206,14 @@ export function format(

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

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

return ''.concat(formatted);
return String(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
: ''.concat(error);
: String(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 (''.concat(+a) === a) {
if (''.concat(+b) !== b) {
if (String(+a) === a) {
if (String(+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(''.concat(data));
return truncateForDisplay(String(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 = ''.concat(val);
value = String(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 = ''.concat(val);
value = String(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(''.concat(children));
expect(textNode.data).toBe(String(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(''.concat(child));
expect(textNode.data).toBe(String(child));
mountIndex++;
} else {
const elementDOMNode = outerNode.childNodes[mountIndex];
Expand Down
16 changes: 7 additions & 9 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(''.concat((expected: any)));
sanitizeURL(String((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 === ''.concat((expected: any))) {
if (value === String((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 === ''.concat((expected: any))) {
} else if (stringValue === String((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 === ''.concat((expected: any))) {
if (value === String((expected: any))) {
return expected;
}
return value;
Expand Down Expand Up @@ -155,9 +155,7 @@ export function setValueForProperty(
} else {
node.setAttribute(
attributeName,
enableTrustedTypesIntegration
? (value: any)
: ''.concat((value: any)),
enableTrustedTypesIntegration ? (value: any) : String((value: any)),
);
}
}
Expand Down Expand Up @@ -189,11 +187,11 @@ export function setValueForProperty(
attributeValue = '';
} else {
// `setAttribute` with objects becomes only `[object]` in IE8/9,
// (''.concat(value)) makes it output the correct toString()-value.
// (String(value)) makes it output the correct toString()-value.
if (enableTrustedTypesIntegration) {
attributeValue = (value: any);
} else {
attributeValue = ''.concat((value: any));
attributeValue = String((value: any));
}
if (propertyInfo.sanitizeURL) {
sanitizeURL(attributeValue.toString());
Expand Down
Loading

0 comments on commit a3c037b

Please sign in to comment.