Skip to content

Commit

Permalink
eslint: Enable @wordpress/no-unused-vars-before-return rule
Browse files Browse the repository at this point in the history
This rule tries to find cases where a variable is set to the result of a
function call, but then an early return happens before that value is
used. The idea being that we can likely save work by not calling a bunch
of functions unnecessarily.

Watch out for functions that have side effects, though. It may be that
those need to be called even if the return value doesn't seem to be
used. React hooks particularly fall into this category; when our
`jetpack-js-tools/eslintrc/react` config is extended any functions
beginning with "use" are ignored as such, but sometimes it seems we're
forgetting to extend that.
  • Loading branch information
anomiex committed Aug 28, 2024
1 parent beee5c1 commit b7534ea
Show file tree
Hide file tree
Showing 81 changed files with 295 additions and 189 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Rearranged some variable assignments for new eslint `@wordpress/no-unused-vars-before-return` rule. Should be no changes to functionality.


3 changes: 1 addition & 2 deletions projects/js-packages/ai-client/src/jwt/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ export default async function requestJwt( {
siteId = siteId || window.JP_CONNECTION_INITIAL_STATE.siteSuffix;
expirationTime = expirationTime || JWT_TOKEN_EXPIRATION_TIME;

const isSimple = isSimpleSite();

// Trying to pick the token from localStorage
const token = localStorage.getItem( JWT_TOKEN_ID );
let tokenData: TokenDataProps | null = null;
Expand All @@ -70,6 +68,7 @@ export default async function requestJwt( {

let data: TokenDataEndpointResponseProps;

const isSimple = isSimpleSite();
if ( ! isSimple ) {
data = await apiFetch( {
/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ export const LogoPresenter: React.FC< LogoPresenterProps > = ( {
logoAccepted = false,
siteId,
} ) => {
// eslint-disable-next-line @wordpress/no-unused-vars-before-return -- @todo Start extending jetpack-js-tools/eslintrc/react in eslintrc, then we can remove this disable comment.
const { isRequestingImage } = useLogoGenerator();
const { saveToLibraryError, logoUpdateError } = useRequestErrors();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,14 @@ export const Prompt: React.FC< { initialPrompt?: string } > = ( { initialPrompt
const onPromptPaste = ( event: React.ClipboardEvent< HTMLInputElement > ) => {
event.preventDefault();

// Paste plain text only
const text = event.clipboardData.getData( 'text/plain' );

const selection = window.getSelection();
if ( ! selection || ! selection.rangeCount ) {
return;
}

// Paste plain text only
const text = event.clipboardData.getData( 'text/plain' );

selection.deleteFromDocument();
const range = selection.getRangeAt( 0 );
range.insertNode( document.createTextNode( text ) );
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Add eslint-disable for a false positive of `@wordpress/no-unused-vars-before-return`.


Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ class I18nCheckPlugin {
* @param {Map} moduleCache - Cache for processed modules.
*/
async #processAsset( compilation, filename, moduleCache ) {
// eslint-disable-next-line @wordpress/no-unused-vars-before-return -- Grabbing current timestamp for timing.
const t0 = Date.now();
const asset = compilation.getAsset( filename );

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Rearranged some variable assignments for new eslint `@wordpress/no-unused-vars-before-return` rule. Should be no changes to functionality.


Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ const StorageHelpPopover = ( { className, forecastInDays } ) => {
const addonSlug = useSelect( select => select( STORE_ID ).getStorageAddonOfferSlug() );
const siteSlug = useSelect( select => select( STORE_ID ).getCalypsoSlug() );
const adminUrl = useSelect( select => select( STORE_ID ).getSiteData().adminUrl );
const storageUpgradeUrl = getProductCheckoutUrl(
addonSlug,
siteSlug,
`${ adminUrl }admin.php?page=jetpack-backup`,
true
);

const popover = useRef( null );

Expand All @@ -44,6 +38,13 @@ const StorageHelpPopover = ( { className, forecastInDays } ) => {
return null;
}

const storageUpgradeUrl = getProductCheckoutUrl(
addonSlug,
siteSlug,
`${ adminUrl }admin.php?page=jetpack-backup`,
true
);

const forecastStatement = sprintf(
/* translators: %d: is number of days of the forecast */
_n(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Rearranged some variable assignments for new eslint `@wordpress/no-unused-vars-before-return` rule. Should be no changes to functionality.


Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ function responsiveVideos() {
video.style.margin = videoMargin;
}

const videoWidth = video.getAttribute( 'data-width' );
const videoHeight = video.getAttribute( 'data-height' );
const videoRatio = video.getAttribute( 'data-ratio' );
const containerWidth = video.parentElement.clientWidth;
Expand All @@ -41,6 +40,8 @@ function responsiveVideos() {
return;
}

const videoWidth = video.getAttribute( 'data-width' );

if ( parseInt( videoWidth, 10 ) > containerWidth ) {
video.style.width = containerWidth + 'px';
video.style.height = containerWidth * parseFloat( videoRatio ) + 'px';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Rearranged some variable assignments for new eslint `@wordpress/no-unused-vars-before-return` rule. Should be no changes to functionality.


Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ window.jetpackForms.generateStyleVariables = function ( formNode ) {
</div>
`;

const _document = window[ 'editor-canvas' ] ? window[ 'editor-canvas' ].document : document;
const bodyNode = _document.querySelector( 'body' );

if ( ! formNode ) {
return;
}

const _document = window[ 'editor-canvas' ] ? window[ 'editor-canvas' ].document : document;
const bodyNode = _document.querySelector( 'body' );

const styleProbe = _document.createElement( 'div' );
styleProbe.className = STYLE_PROBE_CLASS;
styleProbe.style = STYLE_PROBE_STYLE;
Expand Down
8 changes: 4 additions & 4 deletions projects/packages/forms/src/dashboard/inbox/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,16 @@ const InboxResponse = ( { loading, response } ) => {
setTimeout( () => setEmailCopied( false ), 3000 );
}, [ response, setEmailCopied ] );

if ( ! loading && ! response ) {
return null;
}

const titleClasses = clsx( 'jp-forms__inbox-response-title', {
'is-email': response && ! response.author_name && response.author_email,
'is-ip': response && ! response.author_name && ! response.author_email,
'is-name': response && response.author_name,
} );

if ( ! loading && ! response ) {
return null;
}

return (
<SwitchTransition
ref={ ref }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Call `useState` instead of `React.useState` in one place, so it can be seen as a hook call by `@wordpress/no-unused-vars-before-return`. Should be the same function, just returned through `@wordpress/elements`.


Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const SharingModalInner: React.FC = () => {
const [ isOpen, setIsOpen ] = useState( false );
const closeModal = () => setIsOpen( false );
const { createNotice } = useDispatch( noticesStore );
const [ shouldShowSuggestedTags, setShouldShowSuggestedTags ] = React.useState( true );
const [ shouldShowSuggestedTags, setShouldShowSuggestedTags ] = useState( true );

useEffect( () => {
// The first post will show a different modal.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Rearranged some variable assignments for new eslint `@wordpress/no-unused-vars-before-return` rule. Should be no changes to functionality.


7 changes: 4 additions & 3 deletions projects/packages/masterbar/src/admin-menu/admin-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import './admin-menu.css';
*/
function init() {
const adminbar = document.querySelector( '#wpadminbar' );
const wpwrap = document.querySelector( '#wpwrap' );
const adminMenu = document.querySelector( '#adminmenu' );
const dismissClass = 'dismissible-card__close-icon';

if ( ! adminbar ) {
return;
}

const wpwrap = document.querySelector( '#wpwrap' );
const adminMenu = document.querySelector( '#adminmenu' );
const dismissClass = 'dismissible-card__close-icon';

/**
*
* @param value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ export default function RedeemTokenScreen() {
},
} );

const tokenRedeemed = includesLifetimePurchase( purchases );

if ( isLoading ) {
return <>{ __( 'Checking gold status…', 'jetpack-my-jetpack' ) }</>;
}

const tokenRedeemed = includesLifetimePurchase( purchases );

return (
<>
<GoldenTokenModal tokenRedeemed={ tokenRedeemed } displayName={ displayName } />
Expand Down
5 changes: 3 additions & 2 deletions projects/packages/my-jetpack/_inc/utils/format-time.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
type FormatTimeFunction = ( hours: number ) => string;

const formatTime: FormatTimeFunction = ( hours: number ) => {
const seconds = Math.floor( hours * 3600 );
const minutes = Math.floor( seconds / 60 );
hours = Math.floor( hours );
const days = Math.floor( hours / 24 );
const years = Math.floor( days / 365 );
Expand All @@ -15,6 +13,9 @@ const formatTime: FormatTimeFunction = ( hours: number ) => {
return `${ days }d ${ hours % 24 }h`;
}

const seconds = Math.floor( hours * 3600 );
const minutes = Math.floor( seconds / 60 );

if ( hours > 0 ) {
return `${ hours }h ${ minutes % 60 }m`;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Rearranged some variable assignments for new eslint `@wordpress/no-unused-vars-before-return` rule. Should be no changes to functionality.


Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Rearranged some variable assignments for new eslint `@wordpress/no-unused-vars-before-return` rule. Should be no changes to functionality.


Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import jQuery from 'jquery';

jQuery( function ( $ ) {
const state = window.jetpackSocialClassicEditorInitialState ?? {};
const form = $( '#publicize-form' );

if ( ! state || state.sharesRemaining > state.numberOfConnections ) {
return;
Expand All @@ -20,6 +19,7 @@ jQuery( function ( $ ) {
return;
}

const form = $( '#publicize-form' );
form.click( function ( event ) {
const target = $( event.target );

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Rearranged some variable assignments for new eslint `@wordpress/no-unused-vars-before-return` rule. Should be no changes to functionality.


Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ const getNotices = ( tierMaximumRecords = null ) => {
*/
export function NoticeBox( props ) {
const activeNoticeIds = [];
const NOTICES = getNotices( props.tierMaximumRecords );

const DATA_NOT_VALID = '1',
HAS_NOT_BEEN_INDEXED = '2',
Expand All @@ -96,6 +95,7 @@ export function NoticeBox( props ) {
return null;
}

const NOTICES = getNotices( props.tierMaximumRecords );
const notice = NOTICES[ activeNoticeIds[ 0 ] ];

const noticeBoxClassName = notice.isImportant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,6 @@ class SearchApp extends Component {
}

getResultFormat = () => {
// Override the result format from the query string if result_format= is specified
const resultFormatQuery = getResultFormatQuery();

// Override the result format if group static filter is selected, always use expanded.
const isMultiSite =
this.props.staticFilters &&
Expand All @@ -138,6 +135,8 @@ class SearchApp extends Component {
return RESULT_FORMAT_EXPANDED;
}

// Override the result format from the query string if result_format= is specified
const resultFormatQuery = getResultFormatQuery();
return resultFormatQuery || this.state.overlayOptions.resultFormat;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ class SearchResults extends Component {
const { total = 0, corrected_query = false } = this.props.response;
const hasQuery = this.props.searchQuery !== '';
const hasCorrectedQuery = corrected_query !== false;
const num = new Intl.NumberFormat().format( total );
const isMultiSite =
this.props.staticFilters &&
this.props.staticFilters.group_id &&
Expand All @@ -67,6 +66,7 @@ class SearchResults extends Component {
return __( 'No results found', 'jetpack-search-pkg' );
}

const num = new Intl.NumberFormat().format( total );
if ( hasQuery && hasCorrectedQuery ) {
return sprintf(
/* translators: %1$s: number of results. %2$s: the corrected search query. */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Rearranged some variable assignments for new eslint `@wordpress/no-unused-vars-before-return` rule. Should be no changes to functionality.


Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ const UpgradeTrigger = ( { hasUsedVideo = false }: { hasUsedVideo: boolean } ) =
const { adminUri, siteSuffix } = window.jetpackVideoPressInitialState;

const { product, hasVideoPressPurchase, isFetchingPurchases } = usePlan();
// eslint-disable-next-line @wordpress/no-unused-vars-before-return -- @todo Start extending jetpack-js-tools/eslintrc/react in eslintrc, then we can remove this disable comment.
const { run } = useProductCheckoutWorkflow( {
siteSuffix,
productSlug: product.productSlug,
Expand All @@ -337,7 +338,13 @@ const UpgradeTrigger = ( { hasUsedVideo = false }: { hasUsedVideo: boolean } ) =
useBlogIdSuffix: true,
} );

// eslint-disable-next-line @wordpress/no-unused-vars-before-return -- @todo Start extending jetpack-js-tools/eslintrc/react in eslintrc, then we can remove this disable comment.
const { recordEventHandler } = useAnalyticsTracks( {} );

if ( hasVideoPressPurchase || isFetchingPurchases ) {
return null;
}

const onButtonClickHandler = recordEventHandler(
'jetpack_videopress_upgrade_trigger_link_click',
run
Expand All @@ -352,10 +359,6 @@ const UpgradeTrigger = ( { hasUsedVideo = false }: { hasUsedVideo: boolean } ) =
'jetpack-videopress-pkg'
);

if ( hasVideoPressPurchase || isFetchingPurchases ) {
return null;
}

return (
<ContextualUpgradeTrigger
description={ description }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,10 @@ const VideoList = ( {
loading={ loading }
onSelect={ check =>
setSelected( current => {
const indexOf = current.indexOf( index );

if ( check ) {
return [ ...current, index ];
} else if ( ! check && indexOf > -1 ) {
const indexOf = current.indexOf( index );
return [ ...current.slice( 0, indexOf ), ...current.slice( indexOf + 1 ) ];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const VideoStorageMeter: React.FC< VideoStorageMeterProps > = ( {

export const ConnectVideoStorageMeter = props => {
const { storageUsed, uploadedVideoCount } = useVideos();
// eslint-disable-next-line @wordpress/no-unused-vars-before-return -- @todo Start extending jetpack-js-tools/eslintrc/react in eslintrc, then we can remove this disable comment.
const { features } = usePlan();
const { settings } = useVideoPressSettings();
const { siteType } = settings;
Expand Down
Loading

0 comments on commit b7534ea

Please sign in to comment.