Skip to content

Commit

Permalink
Reland Remove redundant initial of isArray (#21188)
Browse files Browse the repository at this point in the history
* Remove redundant initial of isArray (#21163)

* Reapply prettier

* Type the isArray function with refinement support

This ensures that an argument gets refined just like it does if isArray is
used directly.

I'm not sure how to express with just a direct reference so I added a
function wrapper and confirmed that this does get inlined properly by
closure compiler.

* A few more

* Rename unit test to internal

This is not testing a bundle.

Co-authored-by: Behnam Mohammadi <itten@live.com>
  • Loading branch information
sebmarkbage and behnammodi committed Apr 7, 2021
1 parent ee6a05c commit 172e89b
Show file tree
Hide file tree
Showing 38 changed files with 141 additions and 97 deletions.
13 changes: 6 additions & 7 deletions packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ export default {
componentScope = currentScope;
}

const isArray = Array.isArray;

// Next we'll define a few helpers that helps us
// tell if some values don't have to be declared as deps.

Expand All @@ -157,7 +159,7 @@ export default {
// ^^^ true for this reference
// False for everything else.
function isStableKnownHookValue(resolved) {
if (!Array.isArray(resolved.defs)) {
if (!isArray(resolved.defs)) {
return false;
}
const def = resolved.defs[0];
Expand Down Expand Up @@ -226,7 +228,7 @@ export default {
if (
id.type === 'ArrayPattern' &&
id.elements.length === 2 &&
Array.isArray(resolved.identifiers)
isArray(resolved.identifiers)
) {
// Is second tuple value the same reference we're checking?
if (id.elements[1] === resolved.identifiers[0]) {
Expand All @@ -253,10 +255,7 @@ export default {
}
}
} else if (name === 'useTransition') {
if (
id.type === 'ArrayPattern' &&
Array.isArray(resolved.identifiers)
) {
if (id.type === 'ArrayPattern' && isArray(resolved.identifiers)) {
// Is first tuple value the same reference we're checking?
if (id.elements[0] === resolved.identifiers[0]) {
// Setter is stable.
Expand All @@ -270,7 +269,7 @@ export default {

// Some are just functions that don't reference anything dynamic.
function isFunctionWithoutCapturedValues(resolved) {
if (!Array.isArray(resolved.defs)) {
if (!isArray(resolved.defs)) {
return false;
}
const def = resolved.defs[0];
Expand Down
3 changes: 2 additions & 1 deletion packages/jest-react/src/JestReact.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import {REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE} from 'shared/ReactSymbols';

import invariant from 'shared/invariant';
import isArray from 'shared/isArray';

function captureAssertion(fn) {
// Trick to use a Jest matcher inside another Jest matcher. `fn` contains an
Expand Down Expand Up @@ -42,7 +43,7 @@ export function unstable_toMatchRenderedOutput(root, expectedJSX) {
let actualJSX;
if (actualJSON === null || typeof actualJSON === 'string') {
actualJSX = actualJSON;
} else if (Array.isArray(actualJSON)) {
} else if (isArray(actualJSON)) {
if (actualJSON.length === 0) {
actualJSX = null;
} else if (actualJSON.length === 1) {
Expand Down
3 changes: 2 additions & 1 deletion packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ import type {
ElementType,
} from 'react-devtools-shared/src/types';
import is from 'shared/objectIs';
import isArray from 'shared/isArray';

type getDisplayNameForFiberType = (fiber: Fiber) => string | null;
type getTypeSymbolType = (type: any) => Symbol | number;
Expand Down Expand Up @@ -1137,7 +1138,7 @@ export function attach(
memoizedState.hasOwnProperty('create') &&
memoizedState.hasOwnProperty('destroy') &&
memoizedState.hasOwnProperty('deps') &&
(memoizedState.deps === null || Array.isArray(memoizedState.deps)) &&
(memoizedState.deps === null || isArray(memoizedState.deps)) &&
memoizedState.hasOwnProperty('next')
);
}
Expand Down
11 changes: 6 additions & 5 deletions packages/react-devtools-shared/src/backend/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import {copy} from 'clipboard-js';
import {dehydrate} from '../hydration';
import isArray from 'shared/isArray';

import type {DehydratedData} from 'react-devtools-shared/src/devtools/views/Components/types';

Expand Down Expand Up @@ -61,9 +62,9 @@ export function copyWithDelete(
index: number = 0,
): Object | Array<any> {
const key = path[index];
const updated = Array.isArray(obj) ? obj.slice() : {...obj};
const updated = isArray(obj) ? obj.slice() : {...obj};
if (index + 1 === path.length) {
if (Array.isArray(updated)) {
if (isArray(updated)) {
updated.splice(((key: any): number), 1);
} else {
delete updated[key];
Expand All @@ -84,12 +85,12 @@ export function copyWithRename(
index: number = 0,
): Object | Array<any> {
const oldKey = oldPath[index];
const updated = Array.isArray(obj) ? obj.slice() : {...obj};
const updated = isArray(obj) ? obj.slice() : {...obj};
if (index + 1 === oldPath.length) {
const newKey = newPath[index];
// $FlowFixMe number or string is fine here
updated[newKey] = updated[oldKey];
if (Array.isArray(updated)) {
if (isArray(updated)) {
updated.splice(((oldKey: any): number), 1);
} else {
delete updated[oldKey];
Expand All @@ -111,7 +112,7 @@ export function copyWithSet(
return value;
}
const key = path[index];
const updated = Array.isArray(obj) ? obj.slice() : {...obj};
const updated = isArray(obj) ? obj.slice() : {...obj};
// $FlowFixMe number or string is fine here
updated[key] = copyWithSet(obj[key], path, value, index + 1);
return updated;
Expand Down
7 changes: 4 additions & 3 deletions packages/react-dom/src/client/ReactDOMSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {getCurrentFiberOwnerNameInDevOrNull} from 'react-reconciler/src/ReactCur

import {checkControlledValueProps} from '../shared/ReactControlledValuePropTypes';
import {getToStringValue, toString} from './ToStringValue';
import isArray from 'shared/isArray';

let didWarnValueDefaultValue;

Expand Down Expand Up @@ -45,15 +46,15 @@ function checkSelectPropTypes(props) {
if (props[propName] == null) {
continue;
}
const isArray = Array.isArray(props[propName]);
if (props.multiple && !isArray) {
const propNameIsArray = isArray(props[propName]);
if (props.multiple && !propNameIsArray) {
console.error(
'The `%s` prop supplied to <select> must be an array if ' +
'`multiple` is true.%s',
propName,
getDeclarationErrorAddendum(),
);
} else if (!props.multiple && isArray) {
} else if (!props.multiple && propNameIsArray) {
console.error(
'The `%s` prop supplied to <select> must be a scalar ' +
'value if `multiple` is false.%s',
Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/src/client/ReactDOMTextarea.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
*/

import invariant from 'shared/invariant';
import isArray from 'shared/isArray';

import {checkControlledValueProps} from '../shared/ReactControlledValuePropTypes';
import {getCurrentFiberOwnerNameInDevOrNull} from 'react-reconciler/src/ReactCurrentFiber';
import {getToStringValue, toString} from './ToStringValue';
import type {ToStringValue} from './ToStringValue';

import {disableTextareaChildren} from 'shared/ReactFeatureFlags';

let didWarnValDefaultVal = false;
Expand Down Expand Up @@ -100,7 +100,7 @@ export function initWrapperState(element: Element, props: Object) {
defaultValue == null,
'If you supply `defaultValue` on a <textarea>, do not pass children.',
);
if (Array.isArray(children)) {
if (isArray(children)) {
invariant(
children.length <= 1,
'<textarea> can only have at most one child.',
Expand Down
3 changes: 1 addition & 2 deletions packages/react-dom/src/server/ReactDOMServerFormatConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ import hyphenateStyleName from '../shared/hyphenateStyleName';
import invariant from 'shared/invariant';
import hasOwnProperty from 'shared/hasOwnProperty';
import sanitizeURL from '../shared/sanitizeURL';

const isArray = Array.isArray;
import isArray from 'shared/isArray';

// Per response, global state that is not contextual to the rendering subtree.
export type ResponseState = {
Expand Down
11 changes: 6 additions & 5 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {ReactProvider, ReactContext} from 'shared/ReactTypes';

import * as React from 'react';
import invariant from 'shared/invariant';
import isArray from 'shared/isArray';
import getComponentNameFromType from 'shared/getComponentNameFromType';
import {describeUnknownElementTypeFrameInDEV} from 'shared/ReactComponentStackFrame';
import ReactSharedInternals from 'shared/ReactSharedInternals';
Expand Down Expand Up @@ -1438,7 +1439,7 @@ class ReactDOMServerRenderer {
defaultValue == null,
'If you supply `defaultValue` on a <textarea>, do not pass children.',
);
if (Array.isArray(textareaChildren)) {
if (isArray(textareaChildren)) {
invariant(
textareaChildren.length <= 1,
'<textarea> can only have at most one child.',
Expand Down Expand Up @@ -1467,14 +1468,14 @@ class ReactDOMServerRenderer {
if (props[propName] == null) {
continue;
}
const isArray = Array.isArray(props[propName]);
if (props.multiple && !isArray) {
const propNameIsArray = isArray(props[propName]);
if (props.multiple && !propNameIsArray) {
console.error(
'The `%s` prop supplied to <select> must be an array if ' +
'`multiple` is true.',
propName,
);
} else if (!props.multiple && isArray) {
} else if (!props.multiple && propNameIsArray) {
console.error(
'The `%s` prop supplied to <select> must be a scalar ' +
'value if `multiple` is false.',
Expand Down Expand Up @@ -1515,7 +1516,7 @@ class ReactDOMServerRenderer {
value = optionChildren;
}
selected = false;
if (Array.isArray(selectValue)) {
if (isArray(selectValue)) {
// multiple
for (let j = 0; j < selectValue.length; j++) {
if ('' + selectValue[j] === value) {
Expand Down
7 changes: 4 additions & 3 deletions packages/react-dom/src/test-utils/ReactTestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
rethrowCaughtError,
invokeGuardedCallbackAndCatchFirstError,
} from 'shared/ReactErrorUtils';
import isArray from 'shared/isArray';

// Keep in sync with ReactDOM.js, and ReactTestUtilsAct.js:
const EventInternals =
Expand Down Expand Up @@ -97,7 +98,7 @@ function validateClassInstance(inst, methodName) {
}
let received;
const stringified = '' + inst;
if (Array.isArray(inst)) {
if (isArray(inst)) {
received = 'an array';
} else if (inst && inst.nodeType === ELEMENT_NODE && inst.tagName) {
received = 'a DOM node';
Expand Down Expand Up @@ -197,7 +198,7 @@ function scryRenderedDOMComponentsWithClass(root, classNames) {
}
const classList = className.split(/\s+/);

if (!Array.isArray(classNames)) {
if (!isArray(classNames)) {
invariant(
classNames !== undefined,
'TestUtils.scryRenderedDOMComponentsWithClass expects a ' +
Expand Down Expand Up @@ -365,7 +366,7 @@ function executeDispatch(event, listener, inst) {
function executeDispatchesInOrder(event) {
const dispatchListeners = event._dispatchListeners;
const dispatchInstances = event._dispatchInstances;
if (Array.isArray(dispatchListeners)) {
if (isArray(dispatchListeners)) {
for (let i = 0; i < dispatchListeners.length; i++) {
if (event.isPropagationStopped()) {
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
deepDiffer,
flattenStyle,
} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';
import isArray from 'shared/isArray';

import type {AttributeConfiguration} from './ReactNativeTypes';

Expand Down Expand Up @@ -51,7 +52,7 @@ function restoreDeletedValuesInNestedArray(
node: NestedNode,
validAttributes: AttributeConfiguration,
) {
if (Array.isArray(node)) {
if (isArray(node)) {
let i = node.length;
while (i-- && removedKeyCount > 0) {
restoreDeletedValuesInNestedArray(
Expand Down Expand Up @@ -163,12 +164,12 @@ function diffNestedProperty(
return updatePayload;
}

if (!Array.isArray(prevProp) && !Array.isArray(nextProp)) {
if (!isArray(prevProp) && !isArray(nextProp)) {
// Both are leaves, we can diff the leaves.
return diffProperties(updatePayload, prevProp, nextProp, validAttributes);
}

if (Array.isArray(prevProp) && Array.isArray(nextProp)) {
if (isArray(prevProp) && isArray(nextProp)) {
// Both are arrays, we can diff the arrays.
return diffNestedArrayProperty(
updatePayload,
Expand All @@ -178,7 +179,7 @@ function diffNestedProperty(
);
}

if (Array.isArray(prevProp)) {
if (isArray(prevProp)) {
return diffProperties(
updatePayload,
// $FlowFixMe - We know that this is always an object when the input is.
Expand Down Expand Up @@ -212,7 +213,7 @@ function addNestedProperty(
return updatePayload;
}

if (!Array.isArray(nextProp)) {
if (!isArray(nextProp)) {
// Add each property of the leaf.
return addProperties(updatePayload, nextProp, validAttributes);
}
Expand Down Expand Up @@ -242,7 +243,7 @@ function clearNestedProperty(
return updatePayload;
}

if (!Array.isArray(prevProp)) {
if (!isArray(prevProp)) {
// Add each property of the leaf.
return clearProperties(updatePayload, prevProp, validAttributes);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import {invokeGuardedCallbackAndCatchFirstError} from 'shared/ReactErrorUtils';
import invariant from 'shared/invariant';
import isArray from 'shared/isArray';

export let getFiberCurrentPropsFromNode = null;
export let getInstanceFromNode = null;
Expand Down Expand Up @@ -36,14 +37,14 @@ if (__DEV__) {
const dispatchListeners = event._dispatchListeners;
const dispatchInstances = event._dispatchInstances;

const listenersIsArr = Array.isArray(dispatchListeners);
const listenersIsArr = isArray(dispatchListeners);
const listenersLen = listenersIsArr
? dispatchListeners.length
: dispatchListeners
? 1
: 0;

const instancesIsArr = Array.isArray(dispatchInstances);
const instancesIsArr = isArray(dispatchInstances);
const instancesLen = instancesIsArr
? dispatchInstances.length
: dispatchInstances
Expand Down Expand Up @@ -78,7 +79,7 @@ export function executeDispatchesInOrder(event) {
if (__DEV__) {
validateEventDispatches(event);
}
if (Array.isArray(dispatchListeners)) {
if (isArray(dispatchListeners)) {
for (let i = 0; i < dispatchListeners.length; i++) {
if (event.isPropagationStopped()) {
break;
Expand Down Expand Up @@ -106,7 +107,7 @@ function executeDispatchesInOrderStopAtTrueImpl(event) {
if (__DEV__) {
validateEventDispatches(event);
}
if (Array.isArray(dispatchListeners)) {
if (isArray(dispatchListeners)) {
for (let i = 0; i < dispatchListeners.length; i++) {
if (event.isPropagationStopped()) {
break;
Expand Down Expand Up @@ -150,7 +151,7 @@ export function executeDirectDispatch(event) {
const dispatchListener = event._dispatchListeners;
const dispatchInstance = event._dispatchInstances;
invariant(
!Array.isArray(dispatchListener),
!isArray(dispatchListener),
'executeDirectDispatch(...): Invalid `event`.',
);
event.currentTarget = dispatchListener
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import invariant from 'shared/invariant';
import isArray from 'shared/isArray';

/**
* Accumulates items that must not be null or undefined.
Expand All @@ -31,11 +32,11 @@ function accumulate<T>(

// Both are not empty. Warning: Never call x.concat(y) when you are not
// certain that x is an Array (x could be a string with concat method).
if (Array.isArray(current)) {
if (isArray(current)) {
return current.concat(next);
}

if (Array.isArray(next)) {
if (isArray(next)) {
return [current].concat(next);
}

Expand Down
Loading

0 comments on commit 172e89b

Please sign in to comment.