Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove no-fallthrough lint suppressions #26553

Merged
merged 1 commit into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion fixtures/devtools/regression/shared.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable no-fallthrough, react/react-in-jsx-scope, react/jsx-no-undef */
/* eslint-disable react/react-in-jsx-scope, react/jsx-no-undef */
/* global React ReactCache ReactDOM SchedulerTracing ScheduleTracing */

const apps = [];
Expand Down
12 changes: 5 additions & 7 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,6 @@ function setProp(
}
}
// Fall through to the last case which shouldn't remove empty strings.
// eslint-disable-next-line no-fallthrough
case 'formAction': {
if (
value == null ||
Expand Down Expand Up @@ -663,7 +662,7 @@ function setProp(
if (enableCustomElementPropertySupport) {
break;
}
// eslint-disable-next-line no-fallthrough
// Fall through
default: {
if (
key.length > 2 &&
Expand Down Expand Up @@ -761,7 +760,7 @@ function setPropOnCustomElement(
if (enableCustomElementPropertySupport) {
break;
}
// eslint-disable-next-line no-fallthrough
// Fall through
default: {
if (registrationNameDependencies.hasOwnProperty(key)) {
if (__DEV__ && value != null && typeof value !== 'function') {
Expand Down Expand Up @@ -1010,7 +1009,6 @@ export function setInitialProperties(
listenToNonDelegatedEvent('load', domElement);
// We fallthrough to the return of the void elements
}
// eslint-disable-next-line no-fallthrough
case 'area':
case 'base':
case 'br':
Expand Down Expand Up @@ -1174,7 +1172,7 @@ export function diffProperties(
'Cannot update the "is" prop after it has been initialized.',
);
}
// eslint-disable-next-line no-fallthrough
// Fall through
default: {
(updatePayload = updatePayload || []).push(propKey, nextProp);
}
Expand Down Expand Up @@ -1790,7 +1788,7 @@ function diffHydratedCustomComponent(
}
continue;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this continue meant to be unconditional?

Copy link
Collaborator

@sebmarkbage sebmarkbage Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's supposed to only be a continue if the enableCustomElementPropertySupport flag is on, otherwise fallthrough as if they weren't special cased. So this is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohhhhhhhh the 'className' case is empty if enableCustomElementPropertySupport is false. got it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I end up using this technique of putting fall throughs at the end so that the cases themselves get DCE.

}
// eslint-disable-next-line no-fallthrough
// Fall through
case 'className':
if (enableCustomElementPropertySupport) {
// className is a special cased property on the server to render as an attribute.
Expand All @@ -1803,7 +1801,7 @@ function diffHydratedCustomComponent(
warnForPropDifference('className', serverValue, value);
continue;
}
// eslint-disable-next-line no-fallthrough
// Fall through
default: {
let ownNamespaceDev = parentNamespaceDev;
if (ownNamespaceDev === HTML_NAMESPACE) {
Expand Down
12 changes: 4 additions & 8 deletions packages/react-dom-bindings/src/events/ReactDOMEventListener.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,20 +319,17 @@ export function getEventPriority(domEventName: DOMEventName): EventPriority {
case 'touchend':
case 'touchstart':
case 'volumechange':
// Used by polyfills:
// eslint-disable-next-line no-fallthrough
// Used by polyfills: (fall through)
case 'change':
case 'selectionchange':
case 'textInput':
case 'compositionstart':
case 'compositionend':
case 'compositionupdate':
// Only enableCreateEventHandleAPI:
// eslint-disable-next-line no-fallthrough
// Only enableCreateEventHandleAPI: (fall through)
case 'beforeblur':
case 'afterblur':
// Not used by React but could be by user code:
// eslint-disable-next-line no-fallthrough
// Not used by React but could be by user code: (fall through)
case 'beforeinput':
case 'blur':
case 'fullscreenchange':
Expand All @@ -357,8 +354,7 @@ export function getEventPriority(domEventName: DOMEventName): EventPriority {
case 'toggle':
case 'touchmove':
case 'wheel':
// Not used by React but could be by user code:
// eslint-disable-next-line no-fallthrough
// Not used by React but could be by user code: (fall through)
case 'mouseenter':
case 'mouseleave':
case 'pointerenter':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,6 @@ function pushAttribute(
}
}
// Fall through to the last case which shouldn't remove empty strings.
// eslint-disable-next-line no-fallthrough
case 'formAction': {
if (
value == null ||
Expand Down Expand Up @@ -1123,11 +1122,9 @@ function pushStartOption(
case 'dangerouslySetInnerHTML':
innerHTML = propValue;
break;
// eslint-disable-next-line-no-fallthrough
case 'value':
value = propValue;
// We intentionally fallthrough to also set the attribute on the node.
// eslint-disable-next-line-no-fallthrough
default:
pushAttribute(target, propKey, propValue);
break;
Expand Down Expand Up @@ -1248,7 +1245,6 @@ function pushInput(
`${'input'} is a self-closing tag and must neither have \`children\` nor ` +
'use `dangerouslySetInnerHTML`.',
);
// eslint-disable-next-line-no-fallthrough
case 'defaultChecked':
defaultChecked = propValue;
break;
Expand Down Expand Up @@ -1330,7 +1326,6 @@ function pushStartTextArea(
throw new Error(
'`dangerouslySetInnerHTML` does not make sense on <textarea>.',
);
// eslint-disable-next-line-no-fallthrough
default:
pushAttribute(target, propKey, propValue);
break;
Expand Down Expand Up @@ -1677,7 +1672,6 @@ function pushLinkImpl(
`${'link'} is a self-closing tag and must neither have \`children\` nor ` +
'use `dangerouslySetInnerHTML`.',
);
// eslint-disable-next-line-no-fallthrough
default:
pushAttribute(target, propKey, propValue);
break;
Expand Down Expand Up @@ -1906,7 +1900,6 @@ function pushSelfClosing(
`${tag} is a self-closing tag and must neither have \`children\` nor ` +
'use `dangerouslySetInnerHTML`.',
);
// eslint-disable-next-line-no-fallthrough
default:
pushAttribute(target, propKey, propValue);
break;
Expand Down Expand Up @@ -1936,7 +1929,6 @@ function pushStartMenuItem(
throw new Error(
'menuitems cannot have `children` nor `dangerouslySetInnerHTML`.',
);
// eslint-disable-next-line-no-fallthrough
default:
pushAttribute(target, propKey, propValue);
break;
Expand Down Expand Up @@ -2088,7 +2080,6 @@ function pushStartTitle(
throw new Error(
'`dangerouslySetInnerHTML` does not make sense on <title>.',
);
// eslint-disable-next-line-no-fallthrough
default:
pushAttribute(target, propKey, propValue);
break;
Expand Down Expand Up @@ -2787,11 +2778,12 @@ export function pushEndInstance(
if (!enableFloat) {
break;
}
// Fall through
}

// Omitted close tags
// TODO: Instead of repeating this switch we could try to pass a flag from above.
// That would require returning a tuple. Which might be ok if it gets inlined.
// eslint-disable-next-line-no-fallthrough
case 'area':
case 'base':
case 'br':
Expand Down Expand Up @@ -4044,7 +4036,6 @@ function writeStyleResourceDependencyInJS(
`${'link'} is a self-closing tag and must neither have \`children\` nor ` +
'use `dangerouslySetInnerHTML`.',
);
// eslint-disable-next-line-no-fallthrough
default:
writeStyleResourceAttributeInJS(destination, propKey, propValue);
break;
Expand Down Expand Up @@ -4240,7 +4231,6 @@ function writeStyleResourceDependencyInAttr(
`${'link'} is a self-closing tag and must neither have \`children\` nor ` +
'use `dangerouslySetInnerHTML`.',
);
// eslint-disable-next-line-no-fallthrough
default:
writeStyleResourceAttributeInAttr(destination, propKey, propValue);
break;
Expand Down
10 changes: 5 additions & 5 deletions packages/react-reconciler/src/ReactFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -550,29 +550,29 @@ export function createFiberFromTypeAndProps(
if (enableLegacyHidden) {
return createFiberFromLegacyHidden(pendingProps, mode, lanes, key);
}
// eslint-disable-next-line no-fallthrough
// Fall through
case REACT_SCOPE_TYPE:
if (enableScopeAPI) {
return createFiberFromScope(type, pendingProps, mode, lanes, key);
}
// eslint-disable-next-line no-fallthrough
// Fall through
case REACT_CACHE_TYPE:
if (enableCache) {
return createFiberFromCache(pendingProps, mode, lanes, key);
}
// eslint-disable-next-line no-fallthrough
// Fall through
case REACT_TRACING_MARKER_TYPE:
if (enableTransitionTracing) {
return createFiberFromTracingMarker(pendingProps, mode, lanes, key);
}
// eslint-disable-next-line no-fallthrough
// Fall through
case REACT_DEBUG_TRACING_MODE_TYPE:
if (enableDebugTracing) {
fiberTag = Mode;
mode |= DebugTracingMode;
break;
}
// eslint-disable-next-line no-fallthrough
// Fall through
default: {
if (typeof type === 'object' && type !== null) {
switch (type.$$typeof) {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -4096,12 +4096,12 @@ function beginWork(
if (enableFloat && supportsResources) {
return updateHostHoistable(current, workInProgress, renderLanes);
}
// eslint-disable-next-line no-fallthrough
// Fall through
case HostSingleton:
if (enableHostSingletons && supportsSingletons) {
return updateHostSingleton(current, workInProgress, renderLanes);
}
// eslint-disable-next-line no-fallthrough
// Fall through
case HostComponent:
return updateHostComponent(current, workInProgress, renderLanes);
case HostText:
Expand Down
17 changes: 6 additions & 11 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1129,8 +1129,8 @@ function commitLayoutEffectOnFiber(
}
break;
}
// Fall through
}
// eslint-disable-next-line-no-fallthrough
case HostSingleton:
case HostComponent: {
recursivelyTraverseLayoutEffects(
Expand Down Expand Up @@ -1827,8 +1827,8 @@ function commitPlacement(finishedWork: Fiber): void {
insertOrAppendPlacementNode(finishedWork, before, parent);
break;
}
// Fall through
}
// eslint-disable-next-line no-fallthrough
case HostComponent: {
const parent: Instance = parentFiber.stateNode;
if (parentFiber.flags & ContentReset) {
Expand All @@ -1851,7 +1851,6 @@ function commitPlacement(finishedWork: Fiber): void {
insertOrAppendPlacementNodeIntoContainer(finishedWork, before, parent);
break;
}
// eslint-disable-next-line-no-fallthrough
default:
throw new Error(
'Invalid host parent fiber. This error is likely caused by a bug ' +
Expand Down Expand Up @@ -2042,8 +2041,8 @@ function commitDeletionEffectsOnFiber(
}
return;
}
// Fall through
}
// eslint-disable-next-line no-fallthrough
case HostSingleton: {
if (enableHostSingletons && supportsSingletons) {
if (!offscreenSubtreeWasHidden) {
Expand Down Expand Up @@ -2071,15 +2070,14 @@ function commitDeletionEffectsOnFiber(

return;
}
// Fall through
}
// eslint-disable-next-line no-fallthrough
case HostComponent: {
if (!offscreenSubtreeWasHidden) {
safelyDetachRef(deletedFiber, nearestMountedAncestor);
}
// Intentional fallthrough to next branch
}
// eslint-disable-next-line-no-fallthrough
case HostText: {
// We only need to remove the nearest host child. Set the host parent
// to `null` on the stack to indicate that nested children don't
Expand Down Expand Up @@ -2707,8 +2705,8 @@ function commitMutationEffectsOnFiber(
}
return;
}
// Fall through
}
// eslint-disable-next-line-no-fallthrough
case HostSingleton: {
if (enableHostSingletons && supportsSingletons) {
if (flags & Update) {
Expand All @@ -2727,8 +2725,8 @@ function commitMutationEffectsOnFiber(
}
}
}
// Fall through
}
// eslint-disable-next-line-no-fallthrough
case HostComponent: {
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
commitReconciliationEffects(finishedWork);
Expand Down Expand Up @@ -3772,7 +3770,6 @@ function commitPassiveMountOnFiber(
}
// Intentional fallthrough to next branch
}
// eslint-disable-next-line-no-fallthrough
default: {
recursivelyTraversePassiveMountEffects(
finishedRoot,
Expand Down Expand Up @@ -3966,7 +3963,6 @@ export function reconnectPassiveEffects(
}
// Intentional fallthrough to next branch
}
// eslint-disable-next-line-no-fallthrough
default: {
recursivelyTraverseReconnectPassiveEffects(
finishedRoot,
Expand Down Expand Up @@ -4047,7 +4043,6 @@ function commitAtomicPassiveEffects(
}
break;
}
// eslint-disable-next-line-no-fallthrough
default: {
recursivelyTraverseAtomicPassiveEffects(
finishedRoot,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1172,8 +1172,8 @@ function completeWork(
}
}
}
// Fall through
}
// eslint-disable-next-line-no-fallthrough
case HostSingleton: {
if (enableHostSingletons && supportsSingletons) {
popHostContext(workInProgress);
Expand Down Expand Up @@ -1234,8 +1234,8 @@ function completeWork(
bubbleProperties(workInProgress);
return null;
}
// Fall through
}
// eslint-disable-next-line-no-fallthrough
case HostComponent: {
popHostContext(workInProgress);
const type = workInProgress.type;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberThrow.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,8 @@ function throwException(
}
break;
}
// Fall through
}
// eslint-disable-next-line no-fallthrough
default: {
throw new Error(
`Unexpected Suspense handler tag (${suspenseBoundary.tag}). This ` +
Expand Down
4 changes: 0 additions & 4 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -1122,9 +1122,6 @@ function finishConcurrentRender(
case RootFatalErrored: {
throw new Error('Root did not complete. This is a bug in React.');
}
// Flow knows about invariant, so it complains if I add a break
// statement, but eslint doesn't know about invariant, so it complains
// if I do. eslint-disable-next-line no-fallthrough
case RootErrored: {
// We should have already attempted to retry this tree. If we reached
// this point, it errored again. Commit it.
Expand Down Expand Up @@ -2381,7 +2378,6 @@ function replaySuspendedUnitOfWork(unitOfWork: Fiber): void {
unitOfWork.tag = FunctionComponent;
// Fallthrough to the next branch.
}
// eslint-disable-next-line no-fallthrough
case SimpleMemoComponent:
case FunctionComponent: {
// Resolve `defaultProps`. This logic is copied from `beginWork`.
Expand Down
Loading