Clean up some workarounds for old upstream bugs (#40199)

* We no longer need to care about nanoid v3 thinking "browser === esm", because the GB package that had that dep no longer does. * Various `jsx-a11y/label-has-associated-control` ignores referring to jsx-eslint/eslint-plugin-jsx-a11y#869 seem to be fixed now that #39736 requires we use `htmlFor`. The one place still needing an ignore is jsx-eslint/eslint-plugin-jsx-a11y#578 instead. * Remove reference to deleted renovate issue. * Move reference to a Storybook bug to the code actually implementing the workaround. * Update Storybook FAQ reference. * Remove TODO references to a fixed Storybook bug (and fix a wrong prop in one story). * Remove workaround for WordPress/WordPress-Coding-Standards#2390.
Automattic · Nov 18, 2024 · 62e385b · 62e385b
1 parent 3acb7ab
commit 62e385b
Show file tree

Hide file tree

Showing 18 changed files with 72 additions and 30 deletions.
diff --git a/.github/files/renovate-post-upgrade.sh b/.github/files/renovate-post-upgrade.sh
@@ -19,9 +19,7 @@ function die {
 
 # Renovate has a bug where they modify `.npmrc` and don't clean up after themselves,
 # resulting in those modifications being included in the diff.
-# https://github.com/renovatebot/renovate/issues/23528
-# Further, it seems they're reluctant to even admit this is actually a bug, and would
-# rather cast aspersions than collaborate on a fix.
+# https://github.com/renovatebot/renovate/discussions/23489
 # So work around it by manually reverting the file.
 git restore .npmrc
 

diff --git a/projects/js-packages/components/changelog/update-cleanup-some-old-workarounds b/projects/js-packages/components/changelog/update-cleanup-some-old-workarounds
@@ -0,0 +1,5 @@
+Significance: patch
+Type: fixed
+Comment: https://github.com/storybookjs/storybook/issues/7215 was fixed a while back, remove TODOs. And fix a wrong prop in one story.
+
+
diff --git a/projects/js-packages/components/components/action-button/stories/index.stories.jsx b/projects/js-packages/components/components/action-button/stories/index.stories.jsx
@@ -5,17 +5,16 @@ import ActionButton from '../index.jsx';
 export default {
 	title: 'JS Packages/Components/Action Button',
 	component: ActionButton,
-	// TODO: Storybook Actions are not working. See https://github.com/storybookjs/storybook/issues/7215
 	argTypes: {
-		onButtonClick: { action: 'clicked' },
+		onClick: { action: 'clicked' },
 	},
 };
 
 // Export additional stories using pre-defined values
 const Template = args => <ActionButton { ...args } />;
 
 const DefaultArgs = {
-	onButtonClick: action( 'onButtonClick' ),
+	onClick: action( 'onButtonClick' ),
 	displayError: false,
 	isLoading: false,
 	label: 'Action!',

diff --git a/projects/js-packages/components/components/pricing-card/stories/index.stories.tsx b/projects/js-packages/components/components/pricing-card/stories/index.stories.tsx
@@ -5,7 +5,6 @@ import type { StoryFn, Meta } from '@storybook/react';
 export default {
 	title: 'JS Packages/Components/Pricing Card',
 	component: PricingCard,
-	// TODO: Storybook Actions are not working. See https://github.com/storybookjs/storybook/issues/7215
 	argTypes: {
 		onCtaClick: { action: 'clicked' },
 	},

diff --git a/projects/js-packages/publicize-components/changelog/update-cleanup-some-old-workarounds b/projects/js-packages/publicize-components/changelog/update-cleanup-some-old-workarounds
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fixed
+
+Add missing ids to radio buttons in the confirmation form.
diff --git a/.../publicize-components/src/components/manage-connections-modal/confirmation-form/index.tsx b/.../publicize-components/src/components/manage-connections-modal/confirmation-form/index.tsx
@@ -220,10 +220,15 @@ export function ConfirmationForm( { keyringResult, onComplete, isAdmin }: Confir
 									: index === 0;
 
 								return (
-									// eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869
-									<label key={ option.value } className={ styles[ 'account-label' ] } aria-required>
+									<label
+										key={ option.value }
+										htmlFor={ `external_user_ID__${ option.value }` }
+										className={ styles[ 'account-label' ] }
+										aria-required
+									>
 										<input
 											type="radio"
+											id={ `external_user_ID__${ option.value }` }
 											name="external_user_ID"
 											value={ option.value }
 											defaultChecked={ defaultChecked }

diff --git a/projects/js-packages/social-logos/changelog/update-cleanup-some-old-workarounds b/projects/js-packages/social-logos/changelog/update-cleanup-some-old-workarounds
@@ -0,0 +1,4 @@
+Significance: patch
+Type: changed
+
+Update example with ids for jsx-a11y/label-has-associated-control.
diff --git a/projects/js-packages/social-logos/src/react/example.tsx b/projects/js-packages/social-logos/src/react/example.tsx
@@ -68,17 +68,27 @@ function SocialLogosExample() {
 			<div className="display-control-group">
 				<div className="display-control">
 					<h4>Small icons</h4>
-					{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869 */ }
-					<label className="switch">
-						<input type="checkbox" onChange={ handleSmallIconsToggle } checked={ useSmallIcons } />
+					{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/578 */ }
+					<label className="switch" htmlFor="useSmallIcons">
+						<input
+							id="useSmallIcons"
+							type="checkbox"
+							onChange={ handleSmallIconsToggle }
+							checked={ useSmallIcons }
+						/>
 						<span className="handle"></span>
 					</label>
 				</div>
 				<div className="display-control">
 					<h4>Icon names</h4>
-					{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869 */ }
-					<label className="switch">
-						<input type="checkbox" onChange={ handleIconNamesToggle } checked={ showIconNames } />
+					{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/578 */ }
+					<label className="switch" htmlFor="showIconNames">
+						<input
+							id="showIconNames"
+							type="checkbox"
+							onChange={ handleIconNamesToggle }
+							checked={ showIconNames }
+						/>
 						<span className="handle"></span>
 						<span className="switch-label" data-on="On" data-off="Off"></span>
 					</label>

diff --git a/projects/js-packages/storybook/changelog/update-cleanup-some-old-workarounds b/projects/js-packages/storybook/changelog/update-cleanup-some-old-workarounds
@@ -0,0 +1,5 @@
+Significance: patch
+Type: changed
+Comment: Move reference to a Storybook bug to the code specifically for the workaround.
+
+
diff --git a/projects/js-packages/storybook/changelog/update-cleanup-some-old-workarounds#2 b/projects/js-packages/storybook/changelog/update-cleanup-some-old-workarounds#2
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fixed
+
+Update Storybook FAQ reference.
diff --git a/projects/js-packages/storybook/storybook/main.js b/projects/js-packages/storybook/storybook/main.js
@@ -37,8 +37,6 @@ const sbconfig = {
 		'storybook-addon-mock',
 		'@storybook/addon-webpack5-compiler-babel',
 	],
-	// Workaround:
-	// https://github.com/storybookjs/storybook/issues/12270
 	webpackFinal: async config => {
 		// Remove source maps in production builds.
 		if ( process.env.NODE_ENV === 'production' ) {
@@ -63,6 +61,7 @@ const sbconfig = {
 		// Find the DefinePlugin
 		const plugin = config.plugins.find( p => p.definitions?.[ 'process.env' ] );
 		// Add custom env variables
+		// https://github.com/storybookjs/storybook/issues/12270
 		Object.keys( customEnvVariables ).forEach( key => {
 			plugin.definitions[ 'process.env' ][ key ] = JSON.stringify( customEnvVariables[ key ] );
 		} );
@@ -127,7 +126,7 @@ const sbconfig = {
 	},
 	framework: {
 		// Workaround https://github.com/storybookjs/storybook/issues/21710
-		// from https://storybook.js.org/docs/react/faq#how-do-i-fix-module-resolution-while-using-pnpm-plug-n-play
+		// from https://storybook.js.org/docs/faq#how-do-i-fix-module-resolution-in-special-environments
 		name: path.dirname( require.resolve( '@storybook/react-webpack5/package.json' ) ),
 		options: {},
 	},

diff --git a/projects/packages/forms/changelog/update-cleanup-some-old-workarounds b/projects/packages/forms/changelog/update-cleanup-some-old-workarounds
@@ -0,0 +1,5 @@
+Significance: patch
+Type: fixed
+Comment: Remove workaround for fixed WPCS bug.
+
+
diff --git a/projects/packages/forms/src/contact-form/class-admin.php b/projects/packages/forms/src/contact-form/class-admin.php
@@ -1428,8 +1428,7 @@ public function grunion_recheck_queue() {
 		$query = 'post_type=feedback&post_status=publish';
 
 		if ( isset( $_POST['limit'] ) && isset( $_POST['offset'] ) ) {
-			// phpcs:ignore Generic.Strings.UnnecessaryStringConcat.Found -- Avoiding https://github.com/WordPress/WordPress-Coding-Standards/issues/2390
-			$query .= '&posts_per' . '_page=' . (int) $_POST['limit'] . '&offset=' . (int) $_POST['offset'];
+			$query .= '&posts_per_page=' . (int) $_POST['limit'] . '&offset=' . (int) $_POST['offset'];
 		}
 
 		$approved_feedbacks = get_posts( $query );

diff --git a/projects/packages/jetpack-mu-wpcom/changelog/update-cleanup-some-old-workarounds b/projects/packages/jetpack-mu-wpcom/changelog/update-cleanup-some-old-workarounds
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fixed
+
+Add missing ids in Verbum EmailForm.
diff --git a/projects/packages/jetpack-mu-wpcom/changelog/update-cleanup-some-old-workarounds#2 b/projects/packages/jetpack-mu-wpcom/changelog/update-cleanup-some-old-workarounds#2
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fixed
+
+Explicitly set `htmlFor` in recommended tags modal FormLabel.
diff --git a/...packages/jetpack-mu-wpcom/src/features/verbum-comments/src/components/EmailForm/index.tsx b/...packages/jetpack-mu-wpcom/src/features/verbum-comments/src/components/EmailForm/index.tsx
@@ -84,10 +84,10 @@ export const EmailForm = ( { shouldShowEmailForm }: EmailFormProps ) => {
 			{ shouldShowEmailForm && (
 				<div className="verbum-form__wrapper">
 					<div className="verbum-form__content">
-						{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869 */ }
-						<label className="verbum__label">
+						<label htmlFor="verbum-email-form-email" className="verbum__label">
 							<Email />
 							<input
+								id="verbum-email-form-email"
 								className={ clsx( 'verbum-form__email', {
 									'invalid-form-data': isValidEmail.value === false && isEmailTouched.value,
 								} ) }
@@ -108,10 +108,10 @@ export const EmailForm = ( { shouldShowEmailForm }: EmailFormProps ) => {
 							/>
 						</label>
 
-						{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869 */ }
-						<label className="verbum__label">
+						<label htmlFor="verbum-email-form-name" className="verbum__label">
 							<Name />
 							<input
+								id="verbum-email-form-name"
 								className={ clsx( 'verbum-form__name', {
 									'invalid-form-data': isValidAuthor.value === false && isNameTouched.value,
 								} ) }
@@ -130,10 +130,10 @@ export const EmailForm = ( { shouldShowEmailForm }: EmailFormProps ) => {
 							/>
 						</label>
 
-						{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869 */ }
-						<label className="verbum__label">
+						<label htmlFor="verbum-email-form-website" className="verbum__label">
 							<Website />
 							<input
+								id="verbum-email-form-website"
 								className="verbum-form__website"
 								type="text"
 								spellCheck={ false }

diff --git a/...ck-mu-wpcom/src/features/wpcom-block-editor-nux/src/recommended-tags-modal/form-label.tsx b/...ck-mu-wpcom/src/features/wpcom-block-editor-nux/src/recommended-tags-modal/form-label.tsx
@@ -16,13 +16,13 @@ const FormLabel: FunctionComponent< Props & LabelProps > = ( {
 	required,
 	optional,
 	className, // Via LabelProps
+	htmlFor,
 	...labelProps
 } ) => {
 	const hasChildren: boolean = Children.count( children ) > 0;
 
 	return (
-		// eslint-disable-next-line jsx-a11y/label-has-associated-control
-		<label { ...labelProps } className={ clsx( className, 'form-label' ) }>
+		<label htmlFor={ htmlFor } { ...labelProps } className={ clsx( className, 'form-label' ) }>
 			{ children }
 			{ hasChildren && required && (
 				<small className="form-label__required">{ __( 'Required', 'jetpack-mu-wpcom' ) }</small>

diff --git a/tools/js-tools/jest/jest-resolver.js b/tools/js-tools/jest/jest-resolver.js
@@ -2,8 +2,6 @@
 // List them here and the resolver will adjust the conditions to resolve them as "node" instead.
 // cf. https://github.com/microsoft/accessibility-insights-web/pull/5421#issuecomment-1109168149
 const badBrowserPackages = new Set( [
-	// v3 is still supposed to be commonjs-compatible. https://github.com/ai/nanoid/issues/462
-	'nanoid',
 	// https://github.com/LeaVerou/parsel/issues/79
 	'parsel-js',
 ] );