Skip to content

Commit

Permalink
Merge pull request #2083 from woocommerce/release/2.5.5
Browse files Browse the repository at this point in the history
Release 2.5.5
  • Loading branch information
puntope authored Sep 5, 2023
2 parents 51a81fc + 28d9e4f commit 31495f2
Show file tree
Hide file tree
Showing 25 changed files with 888 additions and 404 deletions.
2 changes: 1 addition & 1 deletion .externalized.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
["@woocommerce/components","@woocommerce/customer-effort-score","@woocommerce/data","@woocommerce/navigation","@woocommerce/settings","@wordpress/api-fetch","@wordpress/components","@wordpress/compose","@wordpress/data","@wordpress/data-controls","@wordpress/date","@wordpress/deprecated","@wordpress/dom","@wordpress/element","@wordpress/hooks","@wordpress/html-entities","@wordpress/i18n","@wordpress/primitives","@wordpress/url","lodash","moment","react","react-dom"]
["@woocommerce/components","@woocommerce/currency","@woocommerce/customer-effort-score","@woocommerce/data","@woocommerce/date","@woocommerce/navigation","@woocommerce/number","@woocommerce/settings","@woocommerce/tracks","@wordpress/api-fetch","@wordpress/components","@wordpress/compose","@wordpress/data","@wordpress/data-controls","@wordpress/date","@wordpress/dom","@wordpress/element","@wordpress/hooks","@wordpress/html-entities","@wordpress/i18n","@wordpress/primitives","@wordpress/url","lodash","react","react-dom"]
68 changes: 53 additions & 15 deletions Working with DEWP.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,17 @@ So we could later inspect that.

## Selective bundling & extracting.

Sometimes the fact we do bundle a package that is provided by WordPress/WooCommerce instance introduces errors, as some packages are not effectively modular, so we face version conflicts, style collisions, etc.
Also, we'd like to reduce the size of our bundle, so eventually, we aim to extract/externalize as much as possible and when possible import from an external package.
In the past, we did bundle some packages provided by WordPress/WooCommerce instances. We did so to use a specific package version, for example, to ship a new feature we need, fix, or avoid a bug. However, bundling a package tends to introduce other errors, as some packages are not effectively modular, so we face version conflicts, style collisions, etc.
Also, we'd like to reduce the size of our bundle, so eventually, we aim to extract/externalize as much as possible and import from an external package when possible.

To help with that we implemented the `extracted/` prefix. It's also a custom implementation in [/webpack.config.js](/develop/webpack.config.js).
Thanks to that even though a package is bundled, the given import would fetch it from external.
To help with that, we had implemented the `extracted/` prefix in the past. It was also a custom implementation in [`webpack.config.js`](webpack.config.js).
Thanks to that, even though a package is bundled, the given import would fetch it from external.

Now, we have already externalized all DEWP-able packages, so we also removed the implementation of the `extracted/` prefix.
If someday we ever need it again, please refer to:
- The PR implemented it: https://github.com/woocommerce/google-listings-and-ads/pull/1762
- The commit removed it: https://github.com/woocommerce/google-listings-and-ads/commit/5a2e20409a53ccb3b7fcbfe5c46988ca2b153b38
Alternatively, we can consider an opposite approach and selectively **bundle** with a similar prefix implementation.

## NPM scripts

Expand All @@ -38,18 +44,50 @@ In a regular project to track outdated packages, you would use `npm outdated`. H

### What is the version of the DEWPed package?

As we externalize packages we lose control of the version of the package that will run in the wild (on WordPress instance).
However, with the current platform, we not only lose control, but we also lose the actual information on the range of package we should expect.
So, even if we know we support WooCommerce 6.9+, what are the `@woocommerce/components` there?
As we externalize a package, we lose control of its version that will run in the wild (on WordPress instance).
However, with the current platform, we not only lose control, but we also lose the actual information on the range of package versions we should expect.
So, even if we know we support WooCommerce L-2, what are the `@woocommerce/components` there?

There is no published table of those, and finding them usually requires quite a lot of manual digging. To mitigate that, we created a tool: [`dewped`](https://github.com/woocommerce/dewped#dewped).

First, `latest-versions`/`l-x` helps you check the platform's current, latest, or L-x version:

```bash
$ dewped l-x
Fetching L-2 versions wordpress!
["6.3.1","6.2.2","6.1.3"]

$ dewped l-x woocommerce 3
Fetching L-3 versions woocommerce!
["8.0.3","7.9.0","7.8.2","7.7.2"]
```

Then, with `platform-dependency-version`/`pdep` you may check which version of packages is expected to be present in the platform you target to support and compare it to the locally installed versions.

```bash
$ dewped pdep -w=6.2.2 -c=7.8.2 -d=.externalized.json
Name WordPress 6.2.2 WooCommerce 7.8.2 Local
────────────────────────────────── ─────────────── ───────────────── ────────
@woocommerce/components 12.0.0 ^10.3.0
```


You can also use it to check an individual package. For example, when you consider adding a new dependency and want to check which version to anticipate

```bash
$ dewped pdep --wcVersion=7.8.2 @woocommerce/data
Name WooCommerce 7.8.2 Local
───────────────── ───────────────── ──────
@woocommerce/data 4.1.0 ^4.1.0
```

There is no published table of those, and finding them usually requires quite a lot of manual digging. To mitigate that we created another script.
Please bear in mind there are still dragons:
1. :warning: By the design of DEWP, there is absolutely no guarantee that the package will be there at the version reported by this tool. DEWP makes use of global variables available in runtime. So, any other extension or custom code running in a particular instance can overwrite what you expect in a package.
2. The fact that `dewped pdep package` reports a version of a package does not mean it was actually externalized from your bundle. It only means WP/WC uses a reported version. To check what was effectively externalized, please inspect your Webpack config and DEWP report file (`externalizedReport`).
3. Some packages externalized by DEWP, like `@woocommerce/settings`, are not packages we could find either in npm or in [`woocommerce/woocommerce/packages/js`](https://github.com/woocommerce/woocommerce/commits/trunk/packages/js/) 🤷. There may be no way for you to install them locally or even reason about their versions.
4. The `dewped` tool implementation relies on the internal structure of WordPress and WooCommerce repos, which is not documented, considered API, or even stable. So, it may potentially change at any time, making this tool fail or return invalid results 🤷.

- `npm run dewps:woo 6.9.4` - where `6.9.4` is the version of WooCommerce you would like to check.
(If any of the above caveats bothers you or makes you even even more confused, please refer to https://github.com/WordPress/gutenberg/issues/35630)

Please note this simple script still has several limitations.
1. It works for WooCommerce deps only. WordPress ones are more tricky to get, as the list of packages is less static and regular. Theoretically, we should be able to [use dist-tags](https://github.com/WordPress/gutenberg/issues/24376), like `npm install @wordpress/[email protected]` or `npx wp-scripts packages-update --dist-tag=wp-5.8`.
2. It assumes all packages are prefixed with `@woocommerce/`
3. You need to provide the exact full version. The latest, or `x.y` tree lines are not being resolved automatically.
4. Some packages externalized by DEWP, are not packages we could find neither in npm nor in [`woocommerce/woocommerce/packages/js`](https://github.com/woocommerce/woocommerce/commits/trunk/packages/js/) 🤷
5. It requires at least Node 18 to run.

65 changes: 0 additions & 65 deletions bin/list-woo-dewped.mjs

This file was deleted.

7 changes: 7 additions & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
*** WooCommerce Google Listings and Ads Changelog ***

= 2.5.5 - 2023-09-05 =
* Dev - Add E2E tests - Dashboard - Edit Free Listings.
* Dev - Clean up workarounds for WooCommerce < 6.8.
* Dev - Externalize all WooCommerce JavaScript packages via Dependency Extraction Webpack Plugin (DEWP) and remove the selective bundling implementation that gradually externalizes packages into DEWP.
* Dev - Update DEWP related tools and docs.
* Fix - Fix Taxonomy Attribute Mapping for Product Variations.

= 2.5.4 - 2023-08-29 =
* Dev - Override vulnerability packages: xmlhttprequest-ssl and ws.
* Dev - Update trigger method in Hooks Generator Workflow.
Expand Down
4 changes: 2 additions & 2 deletions google-listings-and-ads.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Plugin Name: Google Listings and Ads
* Plugin URL: https://wordpress.org/plugins/google-listings-and-ads/
* Description: Native integration with Google that allows merchants to easily display their products across Google’s network.
* Version: 2.5.4
* Version: 2.5.5
* Author: WooCommerce
* Author URI: https://woocommerce.com/
* Text Domain: google-listings-and-ads
Expand All @@ -30,7 +30,7 @@

defined( 'ABSPATH' ) || exit;

define( 'WC_GLA_VERSION', '2.5.4' ); // WRCS: DEFINED_VERSION.
define( 'WC_GLA_VERSION', '2.5.5' ); // WRCS: DEFINED_VERSION.
define( 'WC_GLA_MIN_PHP_VER', '7.4' );
define( 'WC_GLA_MIN_WC_VER', '6.9' );

Expand Down
1 change: 0 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ module.exports = {
'\\.scss$': '<rootDir>/tests/mocks/assets/styleMock.js',
// Transform our `.~/` alias.
'^\\.~/(.*)$': '<rootDir>/js/src/$1',
'^extracted/(.*)$': '$1',
'@woocommerce/settings':
'<rootDir>/js/src/tests/dependencies/woocommerce/settings',
'@automattic/calypso-config':
Expand Down
12 changes: 5 additions & 7 deletions js/src/dashboard/all-programs-table-card/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,31 +138,29 @@ const AllProgramsTableCard = ( props ) => {
/>
}
headers={ headers }
rowKey={ ( cells ) => cells[ 0 ].id }
rows={ data.map( ( el ) => {
const isFreeListings = el.id === FREE_LISTINGS_PROGRAM_ID;
const editButtonClassName = classnames( {
[ CAMPAIGN_EDIT_BUTTON_CLASS_NAME ]:
! isFreeListings && ! el.disabledEdit,
} );

// Since the <Table> component uses array index as key to render rows,
// it might cause incorrect state control if a column has an internal state.
// So we have to specific `key` prop on some components of the `display` to work around it.
// Ref: https://github.com/woocommerce/woocommerce-admin/blob/v2.1.2/packages/components/src/table/table.js#L288-L289
// The `id` property in the first cell is for the `rowKey` callback.
return [
{ display: el.title },
{ display: el.title, id: el.id.toString() },
{ display: el.country },
{ display: el.dailyBudget },
{
display: isFreeListings ? (
<FreeListingsDisabledToggle />
) : (
<ProgramToggle key={ el.id } program={ el } />
<ProgramToggle program={ el } />
),
},
{
display: (
<div className="program-actions" key={ el.id }>
<div className="program-actions">
<EditProgramButton
className={ editButtonClassName }
programId={ el.id }
Expand Down
35 changes: 4 additions & 31 deletions js/src/hooks/useNavigateAwayPromptEffect.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { noop } from 'lodash';
const alwaysTrue = () => true;

/**
* Returns a normalized location to handle the inconsistent pathname between history v5 (≥ WC 6.7) and v4 (< WC 6.7).
* Returns a normalized location to handle the inconsistent pathname in history v5 (≥ WC 6.7).
*
* Since WC calls `history.push()` with a path that starts with 'admin.php?...', it brings
* the inconsistent `location` results.
Expand All @@ -19,11 +19,6 @@ const alwaysTrue = () => true;
* @see https://github.com/remix-run/history/blob/v5.3.0/packages/history/index.ts#L701
* @see https://github.com/remix-run/history/blob/v5.3.0/packages/history/index.ts#L1086
*
* The `pathname` in v4 is always '/admin.php'.
*
* @see https://github.com/remix-run/history/blob/v4/modules/createBrowserHistory.js#L166
* @see https://github.com/remix-run/history/blob/v4/modules/LocationUtils.js#L57-L61
*
* @param {Object} location Location object to be normalized.
* @return {Object} Normalized location object.
*/
Expand All @@ -36,7 +31,6 @@ function normalizeLocation( location ) {

/**
* Show prompt when the user tries to unload/leave the page.
* Adds and removed `beforeunload` event listener according to the given flag.
*
* @param {string} message Message to be shown. Note, some browsers may not support this when unloading the page.
* @param {boolean} shouldBlock Boolean flag, whether the prompt should be shown.
Expand All @@ -47,54 +41,33 @@ export default function useNavigateAwayPromptEffect(
shouldBlock,
blockedLocation = alwaysTrue
) {
// history#v5 compatibility: As one of useEffect deps for triggering a new blocking after history is changed.
// history#v5: As one of useEffect deps for triggering a new blocking after history is changed.
const { key } = getHistory().location;

useEffect( () => {
/**
* Bind beforeunload event for non `woocommerce/navigation` links and reloads.
* history#v5 does bind `beforeunload` automatically, with v4 we need to do it ourselves.
*
* @param {Event} e Before Unload Event
*/
const eventListener = ( e ) => {
// If you prevent default behavior in Mozilla Firefox prompt will always be shown.
e.preventDefault();
// Chrome requires returnValue to be set.
e.returnValue = message;
};

let unblock = noop;

if ( shouldBlock ) {
// Block navigation in order to show a confirmation prompt.
unblock = getHistory().block( ( transition ) => {
// In history v4 (< WC 6.7) block method receives two parameter (the location and action).
// In v5 (>= WC 6.7) it has only one parameter that is a transition object with location, retry and action properties.
const { location = transition, retry = noop } = transition;
const { location, retry } = transition;
let shouldUnblock = true;

if ( blockedLocation( normalizeLocation( location ) ) ) {
// Show prompt to confirm if the user wants to navigate away
shouldUnblock = window.confirm( message ); // eslint-disable-line no-alert
}

// v5 compatibility requires the calls to unblock and retry functions.
// history v5 requires the calls to unblock and retry functions.
if ( shouldUnblock ) {
unblock();
retry();
}

// v4 compatibility requires a return boolean to tell whether actually to unblock the navigation.
return shouldUnblock;
} );

window.addEventListener( 'beforeunload', eventListener );
}

return () => {
unblock();
window.removeEventListener( 'beforeunload', eventListener );
};
}, [ key, message, shouldBlock, blockedLocation ] );
}
3 changes: 1 addition & 2 deletions js/src/jsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
"compilerOptions": {
"baseUrl": ".",
"paths": {
".~/*": [ "./*" ],
"extracted/*": [ "*" ]
".~/*": [ "./*" ]
}
}
}
3 changes: 1 addition & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 4 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "google-listings-and-ads",
"title": "Google Listings and Ads",
"version": "2.5.4",
"version": "2.5.5",
"description": "google-listings-and-ads",
"author": "Automattic",
"license": "GPL-3.0-or-later",
Expand Down Expand Up @@ -59,7 +59,6 @@
"@types/jest": "^27.5.2",
"@woocommerce/dependency-extraction-webpack-plugin": "^2.0.0",
"@woocommerce/eslint-plugin": "^1.2.0",
"@wordpress/dependency-extraction-webpack-plugin": "^4.4.0",
"@wordpress/env": "^8.4.0",
"@wordpress/jest-preset-default": "^11.9.0",
"@wordpress/prettier-config": "2.18.1",
Expand Down Expand Up @@ -95,7 +94,6 @@
"check-engines": "wp-scripts check-engines",
"check-licenses": "wp-scripts check-licenses",
"dev": "NODE_ENV=development wp-scripts build",
"dewps:woo": "node bin/list-woo-dewped.mjs",
"doc:tracking": "woocommerce-grow-jsdoc ./js/src",
"format": "wp-scripts format",
"i18n": "WP_CLI_PHP_ARGS='-d memory_limit=2048M' ./vendor/bin/wp i18n make-pot ./ languages/$npm_package_name.pot --slug=$npm_package_name --domain=$npm_package_name --exclude=bin,data,js/src,node_modules,tests,vendor",
Expand All @@ -107,7 +105,7 @@
"lint:php-tests": "vendor/bin/phpcs --standard=tests/phpcs.xml.dist",
"lint:fix:php": "vendor/bin/phpcbf; vendor/bin/phpcbf --standard=tests/phpcs.xml.dist",
"lint:pkg-json": "wp-scripts lint-pkg-json",
"outdated:dewp": "npm outdated `cat .externalized.json | sed 's/[][\",]/ /g'` || true",
"outdated:dewp": "npm outdated --color=always `cat .externalized.json | sed 's/[][\",]/ /g'` | grep -E --color=never \"Depended by|google-listings-and-ads$\" || true",
"outdated:nondewp": "npm outdated --color=always | grep --color=never --invert -E \"^(.\\[31m|.\\[33m)?(`cat .externalized.json | sed 's/[][\"]//g'| sed 's/,/|/g'`)\"",
"packages-update": "wp-scripts packages-update",
"start": "wp-scripts start",
Expand Down Expand Up @@ -145,7 +143,7 @@
},
{
"path": "./js/build/index.js",
"maxSize": "209.08 kB"
"maxSize": "148.52 kB"
},
{
"path": "./js/build/*.css",
Expand All @@ -157,7 +155,7 @@
},
{
"path": "./google-listings-and-ads.zip",
"maxSize": "8.48 mB",
"maxSize": "8.22 mB",
"compression": "none"
}
],
Expand Down
Loading

0 comments on commit 31495f2

Please sign in to comment.