Skip to content

Commit

Permalink
Merge pull request #2517 from woocommerce/dev/2002-use-nodejs-20
Browse files Browse the repository at this point in the history
Upgrade to use Node.js 20 and bump npm dependencies
  • Loading branch information
eason9487 authored Aug 23, 2024
2 parents 5af0314 + c76514b commit 7316de3
Show file tree
Hide file tree
Showing 64 changed files with 23,997 additions and 40,732 deletions.
63 changes: 63 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const jsdocConfig = require( '@wordpress/eslint-plugin/configs/jsdoc' );
const webpackConfig = require( './webpack.config' );

const webpackResolver = {
Expand Down Expand Up @@ -39,19 +40,81 @@ module.exports = {
getComputedStyle: 'readonly',
},
rules: {
'@wordpress/i18n-text-domain': [
'error',
{ allowedTextDomain: 'google-listings-and-ads' },
],
'@wordpress/no-unsafe-wp-apis': 1,
'react/react-in-jsx-scope': 'off',
'react-hooks/exhaustive-deps': [
'warn',
{
additionalHooks: 'useSelect',
},
],
// compatibility-code "WC < 7.6"
//
// Turn it off because:
// - `import { CurrencyFactory } from '@woocommerce/currency';`
// It's supported only since WC 7.6.0
// - `import { userEvent } from '@testing-library/user-event';`
// It works but the official documentation also recommends using the default export
'import/no-named-as-default': 'off',
'jest/expect-expect': [
'warn',
{ assertFunctionNames: [ 'expect', 'expect[A-Z]\\w*' ] },
],
// Turn it off temporarily because it involves a lot of re-alignment. We can revisit it later.
'jsdoc/check-line-alignment': 'off',
// Originally, `@fires` tag indicates that when a method is called, it fires
// a specified type of event that can be listened to, e.g. a native `CustomEvent`.
// The JS package `tracking-jsdoc` changes the definition of the `@fires` tag to
// be able to indicate a tracking event will be sent. Therefore, here we list
// shared `@event` names to avoid false alarms.
'jsdoc/no-undefined-types': [
'error',
{
definedTypes: [
...jsdocConfig.rules[ 'jsdoc/no-undefined-types' ][ 1 ]
.definedTypes,
'gla_datepicker_update',
'gla_documentation_link_click',
'gla_faq',
'gla_filter',
'gla_google_account_connect_button_click',
'gla_google_mc_link_click',
'gla_launch_paid_campaign_button_click',
'gla_mc_account_switch_account_button_click',
'gla_modal_closed',
'gla_modal_open',
'gla_paid_campaign_step',
'gla_setup_ads',
'gla_setup_mc',
'gla_table_go_to_page',
'gla_table_page_click',
],
},
],
},
overrides: [
{
files: [ 'js/src/external-components/woocommerce/**' ],
rules: {
'@wordpress/i18n-text-domain': [
'error',
{ allowedTextDomain: 'woocommerce' },
],
},
},
{
files: [ 'js/src/external-components/wordpress/**' ],
rules: {
'@wordpress/i18n-text-domain': [
'error',
{ allowedTextDomain: '' },
],
},
},
{
files: [ 'tests/e2e/**/*.js' ],
rules: {
Expand Down
5 changes: 5 additions & 0 deletions .npmrc
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
engine-strict=true

# The legacy-peer-deps option eliminates React versioning conflicts in npm peer
# dependencies between @wordpress/* and @woocommerce/*. It should be removed
# after they don't have versioning conflicts.
legacy-peer-deps=true
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v16
v20
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ Now you can build the files using one of these commands:

Notice this repository has `engine-strict=true` directive set. That means you cannot install dependencies with other Node engines rather than the ones defined in the `engines` directive inside [package.json](./package.json). It's recommended to use [NVM](https://github.com/nvm-sh/nvm) and run `nvm use` before installing the dependencies to be sure you're using the recommended Node version.

We added Node `^18` and npm `^9` to allow dependabot to update our dependencies. But these are not supported versions.
The `engines` in package.json includes npm `^9` to allow dependabot to update our dependencies. However, it's not the version intended to be used in development.

- See https://github.com/dependabot/dependabot-core/issues/9277

## Working with DEWP

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,16 @@ const AttributeMappingDeleteRuleModal = ( { onRequestClose = noop, rule } ) => {
};

const handleClose = () => {
if ( deleting ) return;
if ( deleting ) {
return;
}
onRequestClose( 'dismiss' );
};

return (
<AppModal
onRequestClose={ handleClose }
title={ __( 'Delete attribute rule?', ' google-listings-and-ads' ) }
title={ __( 'Delete attribute rule?', 'google-listings-and-ads' ) }
buttons={ [
<AppButton
disabled={ deleting }
Expand Down
8 changes: 5 additions & 3 deletions js/src/attribute-mapping/attribute-mapping-rule-modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ const AttributeMappingRuleModal = ( { rule, onRequestClose = noop } ) => {
};

const handleClose = () => {
if ( saving ) return;
if ( saving ) {
return;
}
onRequestClose( 'dismiss' );
};

Expand All @@ -172,8 +174,8 @@ const AttributeMappingRuleModal = ( { rule, onRequestClose = noop } ) => {
className="gla-attribute-mapping__rule-modal"
title={
rule
? __( 'Manage attribute rule', ' google-listings-and-ads' )
: __( 'Create attribute rule', ' google-listings-and-ads' )
? __( 'Manage attribute rule', 'google-listings-and-ads' )
: __( 'Create attribute rule', 'google-listings-and-ads' )
}
buttons={ [
<AppButton
Expand Down
8 changes: 4 additions & 4 deletions js/src/components/app-button/index.scss
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
.app-button {
&[hidden] {
display: none !important;
}

white-space: nowrap;

svg {
Expand All @@ -14,6 +10,10 @@
}
}

&[hidden] {
display: none !important;
}

// Add a gap between the SVG icon and text when the icon is on the right side.
// `:last-child` is to prevent this style apply on the loading <Spinner>.
&--icon-position-right.has-icon svg:last-child {
Expand Down
14 changes: 7 additions & 7 deletions js/src/components/app-modal/index.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
.app-modal {
/**
* Set `hidden` as the default overflow for container and `auto` for body to
* make them stable across newer versions of @wordpress/components.
* More details can be found in the AppModal implementation.
*/
overflow: hidden;

@include break-large {
width: $gla-width-large;
}
Expand All @@ -19,13 +26,6 @@
}
}

/**
* Set `hidden` as the default overflow for container and `auto` for body to
* make them stable across newer versions of @wordpress/components.
* More details can be found in the AppModal implementation.
*/
overflow: hidden;

.components-modal__content {
overflow: auto;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
}

&__placeholder {
@include placeholder;

display: inline-block;
width: 18em;

@include placeholder;
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { renderHook } from '@testing-library/react-hooks';
import { renderHook } from '@testing-library/react';

/**
* Internal dependencies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import { useRef } from '@wordpress/element';
import { Form } from '@woocommerce/components';
import { pick, noop } from 'lodash';

/**
Expand Down
12 changes: 6 additions & 6 deletions js/src/components/paid-ads/asset-group/images-selector.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe( 'ImagesSelector', () => {
render( <ImagesSelector imageConfig={ imageConfig } /> );
await userEvent.click( getAddButton() );

expect( openSelector ).toBeCalledWith( undefined );
expect( openSelector ).toHaveBeenCalledWith( undefined );
} );

it( 'When clicking on a replace image button, the `openSelector` should be called with the corresponding image ID.', async () => {
Expand All @@ -88,7 +88,7 @@ describe( 'ImagesSelector', () => {
const buttons = getReplaceButtons();
await userEvent.click( buttons.at( 0 ) );

expect( openSelector ).toBeCalledWith( urlA );
expect( openSelector ).toHaveBeenCalledWith( urlA );
} );

it( 'When clicking on a remove image button, the corresponding image should be removed.', async () => {
Expand All @@ -103,7 +103,7 @@ describe( 'ImagesSelector', () => {
await userEvent.click( buttons.at( 1 ) );

expect( getImgUrls() ).toEqual( [ urlA, urlC ] );
expect( onChange ).toBeCalledWith( [ urlA, urlC ] );
expect( onChange ).toHaveBeenCalledWith( [ urlA, urlC ] );
} );

it( 'When the number of images reaches the maximum limit, it should disable the add image button and vice versa.', async () => {
Expand Down Expand Up @@ -195,7 +195,7 @@ describe( 'ImagesSelector', () => {
await act( async () => onSelect( imageB ) );

expect( getImgUrls() ).toEqual( [ urlA, urlB ] );
expect( onChange ).toBeCalledWith( [ urlA, urlB ] );
expect( onChange ).toHaveBeenCalledWith( [ urlA, urlB ] );
} );

it( 'When an image is called back for deletion, the image with the same ID should be removed from the image list.', async () => {
Expand All @@ -209,7 +209,7 @@ describe( 'ImagesSelector', () => {
await act( async () => onDelete( imageB ) );

expect( getImgUrls() ).toEqual( [ urlA, urlC ] );
expect( onChange ).toBeCalledWith( [ urlA, urlC ] );
expect( onChange ).toHaveBeenCalledWith( [ urlA, urlC ] );
} );

it( 'When an image called back for addition already has one with the same ID in the image list, it should not be added.', async () => {
Expand Down Expand Up @@ -238,7 +238,7 @@ describe( 'ImagesSelector', () => {
await act( async () => onSelect( imageC ) );

expect( getImgUrls() ).toEqual( [ urlC, urlB ] );
expect( onChange ).toBeCalledWith( [ urlC, urlB ] );
expect( onChange ).toHaveBeenCalledWith( [ urlC, urlB ] );
} );

it( 'When an image is called back for replacement but the previously clicked image has been removed from the image list, it should be pushed to the image list.', async () => {
Expand Down
4 changes: 3 additions & 1 deletion js/src/components/tree-select-control/checkbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import { Icon, check } from '@wordpress/icons';
* @return {JSX.Element|null} The Checkbox component
*/
const Checkbox = ( { option, checked, className, ...props } ) => {
if ( ! option ) return null;
if ( ! option ) {
return null;
}

return (
<div className={ className }>
Expand Down
4 changes: 3 additions & 1 deletion js/src/components/tree-select-control/control.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ const Control = forwardRef(

const handleKeydown = ( event ) => {
if ( BACKSPACE === event.key ) {
if ( value ) return;
if ( value ) {
return;
}
onTagsChange( tags.slice( 0, -1 ) );
event.preventDefault();
}
Expand Down
54 changes: 30 additions & 24 deletions js/src/components/tree-select-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import { ARROW_DOWN, ARROW_UP, ENTER, ESCAPE, ROOT_VALUE } from './constants';
],
}
],
**/
*/

/**
* @typedef {Object} CommonOption
Expand Down Expand Up @@ -190,7 +190,9 @@ const TreeSelectControl = ( {
const highlightOptionLabel = ( optionLabel, matchPosition ) => {
const matchLength = matchPosition + filter.length;

if ( ! isSearching ) return optionLabel;
if ( ! isSearching ) {
return optionLabel;
}

return (
<span>
Expand Down Expand Up @@ -309,7 +311,9 @@ const TreeSelectControl = ( {
}, [ treeOptions, filter ] );

const onKeyDown = ( event ) => {
if ( disabled ) return;
if ( disabled ) {
return;
}

if ( ESCAPE === event.key ) {
setTreeVisible( false );
Expand Down Expand Up @@ -376,26 +380,6 @@ const TreeSelectControl = ( {
);
};

/**
* Handles a change on the Tree options. Could be a click on a parent option
* or a child option
*
* @param {boolean} checked Indicates if the item should be checked
* @param {InnerOption} option The option to change
*/
const handleOptionsChange = ( checked, option ) => {
if ( option.hasChildren ) {
handleParentChange( checked, option );
} else {
handleSingleChange( checked, option );
}

setInputControlValue( '' );
if ( ! nodesExpanded.includes( option.parent ) ) {
controlRef.current.focus();
}
};

/**
* Handles a change of a child element.
*
Expand Down Expand Up @@ -434,6 +418,26 @@ const TreeSelectControl = ( {
onChange( newValue );
};

/**
* Handles a change on the Tree options. Could be a click on a parent option
* or a child option
*
* @param {boolean} checked Indicates if the item should be checked
* @param {InnerOption} option The option to change
*/
const handleOptionsChange = ( checked, option ) => {
if ( option.hasChildren ) {
handleParentChange( checked, option );
} else {
handleSingleChange( checked, option );
}

setInputControlValue( '' );
if ( ! nodesExpanded.includes( option.parent ) ) {
controlRef.current.focus();
}
};

/**
* Handles a change of a Tag element. We map them to Value format.
*
Expand Down Expand Up @@ -481,7 +485,9 @@ const TreeSelectControl = ( {
setTreeVisible( true );
} }
onControlClick={ () => {
if ( disabled ) return;
if ( disabled ) {
return;
}
setTreeVisible( true );
} }
instanceId={ instanceId }
Expand Down
Loading

0 comments on commit 7316de3

Please sign in to comment.