Skip to content

Commit

Permalink
eslint: Enable more react rules (#39421)
Browse files Browse the repository at this point in the history
* `react-hooks/exhaustive-deps`: Apply to `useSelect`.
* `react/no-direct-mutation-state`
* `react/no-find-dom-node`
* `react/no-unescaped-entities`

The first of those accounts for the majority of fixes in this PR, we had
a bunch of places either missing a dependency or specifying a dependency
unnecessarily. Of particular note is that constants defined outside of
the `render()` function, like `STORE_ID`, don't need to be dependencies.

Of the rest, `react/no-direct-mutation-state` was only violated in one
place, `react/no-unescaped-entities` in one storybook story, and
`react/no-find-dom-node` was already fixed everywhere.
  • Loading branch information
anomiex authored Oct 9, 2024
1 parent 15d9d8c commit f2c8bd3
Show file tree
Hide file tree
Showing 21 changed files with 75 additions and 56 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: fixed
Comment: Remove an unnecessary dep to `useSelect`, and escape apostrophes in a storybook story. Should be no change to functionality.


23 changes: 10 additions & 13 deletions projects/js-packages/components/components/jetpack-footer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,17 @@ const JetpackFooter: React.FC< JetpackFooterProps > = ( {
const [ isMd ] = useBreakpointMatch( 'md', '<=' );
const [ isLg ] = useBreakpointMatch( 'lg', '>' );

const { isActive, connectedPlugins } = useSelect(
select => {
const connectionStatus = select( CONNECTION_STORE_ID ) as {
getConnectedPlugins: () => { slug: string }[];
getConnectionStatus: () => { isActive: boolean };
};
const { isActive, connectedPlugins } = useSelect( select => {
const connectionStatus = select( CONNECTION_STORE_ID ) as {
getConnectedPlugins: () => { slug: string }[];
getConnectionStatus: () => { isActive: boolean };
};

return {
connectedPlugins: connectionStatus?.getConnectedPlugins(),
...connectionStatus.getConnectionStatus(),
};
},
[ CONNECTION_STORE_ID ]
);
return {
connectedPlugins: connectionStatus?.getConnectedPlugins(),
...connectionStatus.getConnectionStatus(),
};
}, [] );
const siteAdminUrl = getSiteAdminUrl();
const areAdminLinksEnabled =
siteAdminUrl &&
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: fixed
Comment: Remove an unnecessary dep to `useSelect`. Should be no change to functionality.


Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default {
// Export additional stories using pre-defined values
const Template = props => (
<ConnectScreenVisual { ...props }>
<p>Secure and speed up your site for free with Jetpack's powerful WordPress tools</p>
<p>Secure and speed up your site for free with Jetpack&apos;s powerful WordPress tools</p>

<ul>
<li>Speed up your site with optimized images</li>
Expand Down Expand Up @@ -52,5 +52,5 @@ Errored.args = {
export const Footer = Template.bind( {} );
Footer.args = {
...DefaultArgs,
footer: <div>Hi I'm a Footer</div>,
footer: <div>Hi I&apos;m a Footer</div>,
};
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export default function useProductCheckoutWorkflow( {
const [ hasCheckoutStarted, setCheckoutStarted ] = useState( false );
const { registerSite } = useDispatch( STORE_ID );

const blogID = useSelect( select => select( STORE_ID ).getBlogId(), [ STORE_ID ] );
const blogID = useSelect( select => select( STORE_ID ).getBlogId(), [] );
debug( 'blogID is %s', blogID ?? 'undefined' );

useBlogIdSuffix = useBlogIdSuffix && !! blogID;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Add missing deps in calls to the `useSelect` React hook.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export function PreviewSection() {
} )
);
},
[ getService, shouldBeDisabled ]
[ canBeTurnedOn, getService, shouldBeDisabled ]
);

const { toggleConnectionById } = useDispatch( socialStore );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export function Threads( { excerpt, title, description, image, url, media } ) {
},
];
},
[ title, image, description, media, url ]
[ excerpt, title, image, description, media, url, shareMessage ]
);

const threadsConnections = useSelect(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Add missing deps in calls to the `useSelect` React hook.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const useModuleStatus = name => {
isLoadingModules: data.areModulesLoading( name ),
};
},
[ JETPACK_MODULES_STORE_ID ]
[ name ]
);

const { updateJetpackModuleStatus } = useDispatch( JETPACK_MODULES_STORE_ID );
Expand Down
4 changes: 2 additions & 2 deletions projects/plugins/jetpack/_inc/client/traffic/site-stats.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class SiteStatsComponent extends React.Component {
countRoles
)
) {
this.state[ `count_roles_${ role }` ] = includes( countRoles, role, false );
this.setState( { [ `count_roles_${ role }` ]: includes( countRoles, role, false ) } );
}
} );
}
Expand All @@ -136,7 +136,7 @@ class SiteStatsComponent extends React.Component {
if (
! [ 'administrator', 'editor', 'author', 'subscriber', 'contributor' ].includes( role )
) {
this.state[ `roles_${ role }` ] = includes( roles, role, false );
this.setState( { [ `roles_${ role }` ]: includes( roles, role, false ) } );
}
} );
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: other

Add missing deps in calls to the `useSelect` React hook.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: other

Use React `setState` instead of directly modifying `state`.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Bullet } from './components';
export const ProgressBullet = ( { key, playerId, index, disabled, isSelected, onClick } ) => {
const progress = useSelect(
select => select( 'jetpack/story/player' ).getCurrentSlideProgressPercentage( playerId ),
[]
[ playerId ]
);

return (
Expand All @@ -25,7 +25,7 @@ export const ProgressBar = ( { playerId, slides, disabled, onSlideSeek, maxBulle
select => ( {
currentSlideIndex: select( 'jetpack/story/player' ).getCurrentSlideIndex( playerId ),
} ),
[]
[ playerId ]
);

const bulletCount = Math.min( slides.length, maxBullets );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const Slide = ( {
currentSlideIndex: select( 'jetpack/story/player' ).getCurrentSlideIndex( playerId ),
buffering: select( 'jetpack/story/player' ).isBuffering( playerId ),
} ),
[]
[ playerId ]
);

const { slideReady } = useDispatch( 'jetpack/story/player' );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const UpgradePlanBanner = ( {

return null;
},
[ description ]
[ description, requiredPlan ]
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const withUpgradeBanner = createHigherOrderComponent(
const bannerContext = 'editor-canvas';
const hasChildrenSelected = useSelect(
select => select( 'core/block-editor' ).hasSelectedInnerBlock( clientId, true ),
[]
[ clientId ]
);
const { hasParentBanner } = useContext( PaidBlockContext ) || {};
// Banner should be not be displayed if one of its parents is already displaying a banner.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,22 +108,19 @@ function AiPostExcerpt() {
}, [ stopSuggestion, reset ] );

// Pick raw post content
const postContent = useSelect(
select => {
const content = ( select( editorStore ) as typeof EditorSelectors ).getEditedPostContent();
if ( ! content ) {
return '';
}
const postContent = useSelect( select => {
const content = ( select( editorStore ) as typeof EditorSelectors ).getEditedPostContent();
if ( ! content ) {
return '';
}

const document = new window.DOMParser().parseFromString( content, 'text/html' );
const document = new window.DOMParser().parseFromString( content, 'text/html' );

const documentRawText = document.body.textContent || document.body.innerText || '';
const documentRawText = document.body.textContent || document.body.innerText || '';

// Keep only one break line (\n) between blocks.
return documentRawText.replace( /\n{2,}/g, '\n' ).trim();
},
[ postId ]
);
// Keep only one break line (\n) between blocks.
return documentRawText.replace( /\n{2,}/g, '\n' ).trim();
}, [] );

// Show custom prompt number of words
const currentExcerpt = suggestion || excerpt;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default function ProductManagementControls( {
selectedProductIds,
setSelectedProductIds
),
[]
[ productType, selectedProductIds, setSelectedProductIds ]
);

const { connectUrl, isApiConnected, areSelectedProductsInvalid } = useSelect( select => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,21 @@ import useAutosaveAndRedirect from '../use-autosave-and-redirect/index';
const HOOK_OPEN_CHECKOUT_MODAL = 'a8c.wpcom-block-editor.openCheckoutModal';

export default function useUpgradeFlow( planSlug, onRedirect = noop ) {
const { checkoutUrl, planData } = useSelect( select => {
const editorSelector = select( 'core/editor' );
const planSelector = select( 'wordpress-com/plans' );

const { id: postId, type: postType } = editorSelector.getCurrentPost();
const plan = planSelector && planSelector.getPlan( planSlug );

return {
checkoutUrl: getUpgradeUrl( { plan, planSlug, postId, postType } ),
planData: plan,
};
}, [] );
const { checkoutUrl, planData } = useSelect(
select => {
const editorSelector = select( 'core/editor' );
const planSelector = select( 'wordpress-com/plans' );

const { id: postId, type: postType } = editorSelector.getCurrentPost();
const plan = planSelector && planSelector.getPlan( planSlug );

return {
checkoutUrl: getUpgradeUrl( { plan, planSlug, postId, postType } ),
planData: plan,
};
},
[ planSlug ]
);

const { autosave, autosaveAndRedirect, isRedirecting } = useAutosaveAndRedirect(
checkoutUrl,
Expand Down
4 changes: 0 additions & 4 deletions tools/js-tools/eslintrc/react.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ module.exports = {
'react/prefer-es6-class': 'warn',

// Temporarily override plugin:@wordpress/react so we can clean up failing stuff in separate PRs.
'react-hooks/exhaustive-deps': [ 'warn', { additionalHooks: '' } ],
'react/jsx-key': 'off',
'react/no-direct-mutation-state': 'off',
'react/no-find-dom-node': 'off',
'react/no-unescaped-entities': 'off',
},
};

0 comments on commit f2c8bd3

Please sign in to comment.