Skip to content

Commit 008ca28

Browse files
committed
Don't report lint error if coercion is safe
This commit extends the safe-string-coercion internal ESLint rule to auto-detect code where coercion can't fail or is correctly preceded by a DEV check. In those cases, the rule now won't report. Changes: * Don't report if the coercion is wrapped in an `if (typeof...) {}` block that ensures that the coercion won't throw * Don't report if there's a valid coercion check call in the statement immediately before the coercion. The check call must be in the following format: if (__DEV__) { checkXxxxxStringCoercion(value); }; * Renames rule option for clarity. New name: `isProductionUserAppCode` * Removes now-unnecessary `eslint-disable*` comments * Improve messages returned from ESLint when the DEV check is required, but is missing or invalid. If invalid, message tells what's wrong.
1 parent 585ed30 commit 008ca28

35 files changed

+549
-133
lines changed

.eslintrc.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ module.exports = {
110110
'react-internal/no-primitive-constructors': ERROR,
111111
'react-internal/safe-string-coercion': [
112112
ERROR,
113-
{disallowStringConstructor: true},
113+
{isProductionUserAppCode: true},
114114
],
115115
'react-internal/no-to-warn-dev-within-to-throw': ERROR,
116116
'react-internal/invariant-args': ERROR,
@@ -181,7 +181,7 @@ module.exports = {
181181
'react-internal/warning-args': OFF,
182182
'react-internal/safe-string-coercion': [
183183
ERROR,
184-
{disallowStringConstructor: false},
184+
{isProductionUserAppCode: false},
185185
],
186186

187187
// Disable accessibility checks

packages/react-devtools-core/src/editor.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ function getArgumentsForLineNumber(
6363
case 'rmate':
6464
case 'mate':
6565
case 'mine':
66-
// eslint-disable-next-line react-internal/safe-string-coercion
6766
return ['--line', lineNumber + '', filePath];
6867
case 'code':
6968
return ['-g', filePath + ':' + lineNumber];

packages/react-devtools-extensions/src/background.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ chrome.runtime.onConnect.addListener(function(port) {
3232
});
3333

3434
function isNumeric(str: string): boolean {
35-
// eslint-disable-next-line react-internal/safe-string-coercion
3635
return +str + '' === str;
3736
}
3837

packages/react-devtools-extensions/src/main.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ function createPanelIfReactLoaded() {
8989

9090
function initBridgeAndStore() {
9191
const port = chrome.runtime.connect({
92-
// eslint-disable-next-line react-internal/safe-string-coercion
9392
name: '' + tabId,
9493
});
9594
// Looks like `port.onDisconnect` does not trigger on in-tab navigation like new URL or back/forward navigation,

packages/react-dom/src/client/DOMPropertyOperations.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ export function getValueForProperty(
4141
const {propertyName} = propertyInfo;
4242
return (node: any)[propertyName];
4343
} else {
44+
// This check protects multiple uses of `expected`, which is why the
45+
// react-internal/safe-string-coercion rule is disabled in several spots
46+
// below.
4447
if (__DEV__) {
4548
checkAttributeStringCoercion(expected, name);
4649
}
@@ -130,7 +133,6 @@ export function getValueForAttribute(
130133
if (__DEV__) {
131134
checkAttributeStringCoercion(expected, name);
132135
}
133-
// eslint-disable-next-line react-internal/safe-string-coercion
134136
if (value === '' + (expected: any)) {
135137
return expected;
136138
}
@@ -170,7 +172,6 @@ export function setValueForProperty(
170172
}
171173
node.setAttribute(
172174
attributeName,
173-
// eslint-disable-next-line react-internal/safe-string-coercion
174175
enableTrustedTypesIntegration ? (value: any) : '' + (value: any),
175176
);
176177
}
@@ -210,7 +211,6 @@ export function setValueForProperty(
210211
if (__DEV__) {
211212
checkAttributeStringCoercion(value, attributeName);
212213
}
213-
// eslint-disable-next-line react-internal/safe-string-coercion
214214
attributeValue = '' + (value: any);
215215
}
216216
if (propertyInfo.sanitizeURL) {

packages/react-dom/src/client/ReactDOMComponent.js

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ if (__DEV__) {
144144
checkHtmlStringCoercion(markup);
145145
}
146146
const markupString =
147-
// eslint-disable-next-line react-internal/safe-string-coercion
148147
typeof markup === 'string' ? markup : '' + (markup: any);
149148
return markupString
150149
.replace(NORMALIZE_NEWLINES_REGEX, '\n')
@@ -308,7 +307,6 @@ function setInitialDOMProperties(
308307
setTextContent(domElement, nextProp);
309308
}
310309
} else if (typeof nextProp === 'number') {
311-
// eslint-disable-next-line react-internal/safe-string-coercion
312310
setTextContent(domElement, '' + nextProp);
313311
}
314312
} else if (
@@ -754,7 +752,6 @@ export function diffProperties(
754752
}
755753
} else if (propKey === CHILDREN) {
756754
if (typeof nextProp === 'string' || typeof nextProp === 'number') {
757-
// eslint-disable-next-line react-internal/safe-string-coercion
758755
(updatePayload = updatePayload || []).push(propKey, '' + nextProp);
759756
}
760757
} else if (
@@ -988,12 +985,10 @@ export function diffHydratedProperties(
988985
updatePayload = [CHILDREN, nextProp];
989986
}
990987
} else if (typeof nextProp === 'number') {
991-
// eslint-disable-next-line react-internal/safe-string-coercion
992988
if (domElement.textContent !== '' + nextProp) {
993989
if (__DEV__ && !suppressHydrationWarning) {
994990
warnForTextDifference(domElement.textContent, nextProp);
995991
}
996-
// eslint-disable-next-line react-internal/safe-string-coercion
997992
updatePayload = [CHILDREN, '' + nextProp];
998993
}
999994
}

packages/react-dom/src/client/ReactDOMHostConfig.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,6 @@ export function createInstance(
275275
typeof props.children === 'string' ||
276276
typeof props.children === 'number'
277277
) {
278-
// eslint-disable-next-line react-internal/safe-string-coercion
279278
const string = '' + props.children;
280279
const ownAncestorInfo = updatedAncestorInfo(
281280
hostContextDev.ancestorInfo,
@@ -331,7 +330,6 @@ export function prepareUpdate(
331330
(typeof newProps.children === 'string' ||
332331
typeof newProps.children === 'number')
333332
) {
334-
// eslint-disable-next-line react-internal/safe-string-coercion
335333
const string = '' + newProps.children;
336334
const ownAncestorInfo = updatedAncestorInfo(
337335
hostContextDev.ancestorInfo,

packages/react-dom/src/client/ReactDOMInput.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,6 @@ function updateNamedCousins(rootNode, props) {
370370
checkAttributeStringCoercion(name, 'name');
371371
}
372372
const group = queryRoot.querySelectorAll(
373-
// eslint-disable-next-line react-internal/safe-string-coercion
374373
'input[name=' + JSON.stringify('' + name) + '][type="radio"]',
375374
);
376375

packages/react-dom/src/client/ToStringValue.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export opaque type ToStringValue =
2121
// around this limitation, we use an opaque type that can only be obtained by
2222
// passing the value through getToStringValue first.
2323
export function toString(value: ToStringValue): string {
24+
// The coercion safety check is performed in getToStringValue().
2425
// eslint-disable-next-line react-internal/safe-string-coercion
2526
return '' + (value: any);
2627
}

packages/react-dom/src/client/inputValueTracking.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ function trackValueOnNode(node: any): ?ValueTracker {
6060
if (__DEV__) {
6161
checkFormFieldValueStringCoercion(node[valueField]);
6262
}
63-
// eslint-disable-next-line react-internal/safe-string-coercion
6463
let currentValue = '' + node[valueField];
6564

6665
// if someone has already defined a value or Safari, then bail
@@ -85,7 +84,6 @@ function trackValueOnNode(node: any): ?ValueTracker {
8584
if (__DEV__) {
8685
checkFormFieldValueStringCoercion(value);
8786
}
88-
// eslint-disable-next-line react-internal/safe-string-coercion
8987
currentValue = '' + value;
9088
set.call(this, value);
9189
},
@@ -106,7 +104,6 @@ function trackValueOnNode(node: any): ?ValueTracker {
106104
if (__DEV__) {
107105
checkFormFieldValueStringCoercion(value);
108106
}
109-
// eslint-disable-next-line react-internal/safe-string-coercion
110107
currentValue = '' + value;
111108
},
112109
stopTracking() {

0 commit comments

Comments
 (0)