Skip to content

Commit

Permalink
Use .slice() for all substring-ing (facebook#26677)
Browse files Browse the repository at this point in the history
- substr is Annex B
- substring silently flips its arguments if they're in the "wrong order", which is confusing
- slice is better than sliced bread (no pun intended) and also it works the same way on Arrays so there's less to remember

---

> I'd be down to just lint and enforce a single form just for the potential compression savings by using a repeated string.

_Originally posted by @sebmarkbage in facebook#26663 (comment)
  • Loading branch information
sophiebits authored and AndyPengc12 committed Apr 15, 2024
1 parent 436af43 commit c15774a
Show file tree
Hide file tree
Showing 35 changed files with 97 additions and 90 deletions.
9 changes: 8 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,14 @@ module.exports = {
'no-inner-declarations': [ERROR, 'functions'],
'no-multi-spaces': ERROR,
'no-restricted-globals': [ERROR].concat(restrictedGlobals),
'no-restricted-syntax': [ERROR, 'WithStatement'],
'no-restricted-syntax': [
ERROR,
'WithStatement',
{
selector: 'MemberExpression[property.name=/^(?:substring|substr)$/]',
message: 'Prefer string.slice() over .substring() and .substr().',
},
],
'no-shadow': ERROR,
'no-unused-vars': [ERROR, {args: 'none'}],
'no-use-before-define': OFF,
Expand Down
2 changes: 1 addition & 1 deletion fixtures/concurrent/time-slicing/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class App extends PureComponent {
}
const multiplier = input.length !== 0 ? input.length : 1;
const complexity =
(parseInt(window.location.search.substring(1), 10) / 100) * 25 || 25;
(parseInt(window.location.search.slice(1), 10) / 100) * 25 || 25;
const data = _.range(5).map(t =>
_.range(complexity * multiplier).map((j, i) => {
return {
Expand Down
2 changes: 1 addition & 1 deletion fixtures/dom/src/react-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import semver from 'semver';

function parseQuery(qstr) {
var query = {};
var a = qstr.substr(1).split('&');
var a = qstr.slice(1).split('&');

for (var i = 0; i < a.length; i++) {
var b = a[i].split('=');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const ReactHooksESLintRule = ReactHooksESLintPlugin.rules['exhaustive-deps'];
function normalizeIndent(strings) {
const codeLines = strings[0].split('\n');
const leftPadding = codeLines[1].match(/\s+/)[0];
return codeLines.map(line => line.substr(leftPadding.length)).join('\n');
return codeLines.map(line => line.slice(leftPadding.length)).join('\n');
}

// ***************************************************
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ ESLintTester.setDefaultConfig({
function normalizeIndent(strings) {
const codeLines = strings[0].split('\n');
const leftPadding = codeLines[1].match(/\s+/)[0];
return codeLines.map(line => line.substr(leftPadding.length)).join('\n');
return codeLines.map(line => line.slice(leftPadding.length)).join('\n');
}

// ***************************************************
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
Original file line number Diff line number Diff line change
Expand Up @@ -1103,7 +1103,7 @@ export default {
extraWarning =
` You can also do a functional update '${
setStateRecommendation.setter
}(${setStateRecommendation.missingDep.substring(
}(${setStateRecommendation.missingDep.slice(
0,
1,
)} => ...)' if you only need '${
Expand Down
18 changes: 9 additions & 9 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,33 +515,33 @@ export function parseModelString(
switch (value[1]) {
case '$': {
// This was an escaped string value.
return value.substring(1);
return value.slice(1);
}
case 'L': {
// Lazy node
const id = parseInt(value.substring(2), 16);
const id = parseInt(value.slice(2), 16);
const chunk = getChunk(response, id);
// We create a React.lazy wrapper around any lazy values.
// When passed into React, we'll know how to suspend on this.
return createLazyChunkWrapper(chunk);
}
case '@': {
// Promise
const id = parseInt(value.substring(2), 16);
const id = parseInt(value.slice(2), 16);
const chunk = getChunk(response, id);
return chunk;
}
case 'S': {
// Symbol
return Symbol.for(value.substring(2));
return Symbol.for(value.slice(2));
}
case 'P': {
// Server Context Provider
return getOrCreateServerContext(value.substring(2)).Provider;
return getOrCreateServerContext(value.slice(2)).Provider;
}
case 'F': {
// Server Reference
const id = parseInt(value.substring(2), 16);
const id = parseInt(value.slice(2), 16);
const chunk = getChunk(response, id);
switch (chunk.status) {
case RESOLVED_MODEL:
Expand Down Expand Up @@ -582,15 +582,15 @@ export function parseModelString(
}
case 'D': {
// Date
return new Date(Date.parse(value.substring(2)));
return new Date(Date.parse(value.slice(2)));
}
case 'n': {
// BigInt
return BigInt(value.substring(2));
return BigInt(value.slice(2));
}
default: {
// We assume that anything else is a reference ID.
const id = parseInt(value.substring(1), 16);
const id = parseInt(value.slice(1), 16);
const chunk = getChunk(response, id);
switch (chunk.status) {
case RESOLVED_MODEL:
Expand Down
12 changes: 6 additions & 6 deletions packages/react-client/src/ReactFlightClientStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,19 @@ function processFullRow(response: Response, row: string): void {
return;
}
const colon = row.indexOf(':', 0);
const id = parseInt(row.substring(0, colon), 16);
const id = parseInt(row.slice(0, colon), 16);
const tag = row[colon + 1];
// When tags that are not text are added, check them here before
// parsing the row as text.
// switch (tag) {
// }
switch (tag) {
case 'I': {
resolveModule(response, id, row.substring(colon + 2));
resolveModule(response, id, row.slice(colon + 2));
return;
}
case 'E': {
const errorInfo = JSON.parse(row.substring(colon + 2));
const errorInfo = JSON.parse(row.slice(colon + 2));
if (__DEV__) {
resolveErrorDev(
response,
Expand All @@ -63,7 +63,7 @@ function processFullRow(response: Response, row: string): void {
}
default: {
// We assume anything else is JSON.
resolveModel(response, id, row.substring(colon + 1));
resolveModel(response, id, row.slice(colon + 1));
return;
}
}
Expand All @@ -76,13 +76,13 @@ export function processStringChunk(
): void {
let linebreak = chunk.indexOf('\n', offset);
while (linebreak > -1) {
const fullrow = response._partialRow + chunk.substring(offset, linebreak);
const fullrow = response._partialRow + chunk.slice(offset, linebreak);
processFullRow(response, fullrow);
response._partialRow = '';
offset = linebreak + 1;
linebreak = chunk.indexOf('\n', offset);
}
response._partialRow += chunk.substring(offset);
response._partialRow += chunk.slice(offset);
}

export function processBinaryChunk(
Expand Down
4 changes: 2 additions & 2 deletions packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -513,10 +513,10 @@ function parseCustomHookName(functionName: void | string): string {
if (startIndex === -1) {
startIndex = 0;
}
if (functionName.substr(startIndex, 3) === 'use') {
if (functionName.slice(startIndex, startIndex + 3) === 'use') {
startIndex += 3;
}
return functionName.substr(startIndex);
return functionName.slice(startIndex);
}

function buildTree(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ describe('ReactHooksInspectionIntegration', () => {
const Suspense = React.Suspense;

function Foo(props) {
const [value] = React.useState(props.defaultValue.substr(0, 3));
const [value] = React.useState(props.defaultValue.slice(0, 3));
return <div>{value}</div>;
}
Foo.defaultProps = {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-extensions/deploy.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const main = async buildId => {
const json = JSON.parse(file);
const alias = json.alias[0];

const commit = execSync('git rev-parse HEAD').toString().trim().substr(0, 7);
const commit = execSync('git rev-parse HEAD').toString().trim().slice(0, 7);

let date = new Date();
date = `${date.toLocaleDateString()}${date.toLocaleTimeString()}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function serializeHook(hook) {
// Remove user-specific portions of this file path.
let fileName = hook.hookSource.fileName;
const index = fileName.lastIndexOf('/react-devtools-shared/');
fileName = fileName.substring(index + 1);
fileName = fileName.slice(index + 1);

let subHooks = hook.subHooks;
if (subHooks) {
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 @@ -4315,7 +4315,7 @@ export function attach(
if (pseudoKey === undefined) {
throw new Error('Expected root pseudo key to be known.');
}
const name = pseudoKey.substring(0, pseudoKey.lastIndexOf(':'));
const name = pseudoKey.slice(0, pseudoKey.lastIndexOf(':'));
const counter = rootDisplayNameCounter.get(name);
if (counter === undefined) {
throw new Error('Expected counter to be known.');
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/devtools/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export function sanitizeForParse(value: any): any | string {
value.charAt(0) === "'" &&
value.charAt(value.length - 1) === "'"
) {
return '"' + value.substr(1, value.length - 2) + '"';
return '"' + value.slice(1, value.length - 1) + '"';
}
}
return value;
Expand Down
8 changes: 4 additions & 4 deletions packages/react-devtools-shared/src/devtools/views/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ export function createRegExp(string: string): RegExp {
// Allow /regex/ syntax with optional last /
if (string[0] === '/') {
// Cut off first slash
string = string.substring(1);
string = string.slice(1);
// Cut off last slash, but only if it's there
if (string[string.length - 1] === '/') {
string = string.substring(0, string.length - 1);
string = string.slice(0, string.length - 1);
}
try {
return new RegExp(string, 'i');
Expand Down Expand Up @@ -186,9 +186,9 @@ export function truncateText(text: string, maxLength: number): string {
const {length} = text;
if (length > maxLength) {
return (
text.substr(0, Math.floor(maxLength / 2)) +
text.slice(0, Math.floor(maxLength / 2)) +
'…' +
text.substr(length - Math.ceil(maxLength / 2) - 1)
text.slice(length - Math.ceil(maxLength / 2) - 1)
);
} else {
return text;
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 @@ -693,7 +693,7 @@ function truncateForDisplay(
length: number = MAX_PREVIEW_STRING_LENGTH,
) {
if (string.length > length) {
return string.substr(0, length) + '…';
return string.slice(0, length) + '…';
} else {
return string;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-timeline/src/EventTooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ const TooltipNetworkMeasure = ({
let urlToDisplay = url;
if (urlToDisplay.length > MAX_TOOLTIP_TEXT_LENGTH) {
const half = Math.floor(MAX_TOOLTIP_TEXT_LENGTH / 2);
urlToDisplay = url.substr(0, half) + '…' + url.substr(url.length - half);
urlToDisplay = url.slice(0, half) + '…' + url.slice(url.length - half);
}

const timestampBegin = sendRequestTimestamp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function trimText(
while (startIndex <= stopIndex) {
const currentIndex = Math.floor((startIndex + stopIndex) / 2);
const trimmedText =
currentIndex === maxIndex ? text : text.substr(0, currentIndex) + '…';
currentIndex === maxIndex ? text : text.slice(0, currentIndex) + '…';

if (getTextWidth(context, trimmedText) <= width) {
if (longestValidIndex < currentIndex) {
Expand Down
Loading

0 comments on commit c15774a

Please sign in to comment.