Update WordPress dependencies #1135

noahtallen · 2021-09-10T00:02:35Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Over at wp-calypso, we're updating to React 17. This means getting our peer dependencies updated to the right versions, and it turns out that newspack-components is a peer dep of newspack-blocks, which we sync to WordPress.com via Calypso.

This PR updates to React 17 and to the latest WordPress component packages.

Also updates most NPM packages to the latest compatible versions.

Also specifies Node version support as v16.11.1, matching Calypso's Node support.

Also enables Dependabot to keep NPM and Composer packages up-to-date.

Bonus: Also fixes a minor pet peeve of mine, which is that this repo's npm run release:archive script previously built the plugin archive ZIP to the assets/release directory, whereas every other Newspack plugin repo builds to ./release (in the root). Now this repo should also build to the root ./release directory.

How to test the changes in this Pull Request:

Using NVM, set your current Node version to v16.11.1 (matching Calypso's supported version)
Ensure npm ci runs
Ensure all NPM build, lint and test scripts still run (build, dev, lint, test)
Test newspack components (see Components Demo page)
Test newspack plugin (especially wizard pages)

Possible issues/follow-up

~~Dependency react-daterange-picker is stale and doesn't appear to be updated anymore. It doesn't list react 17 support, but I suppose it probably supports it anyways.~~ The package still works in the Campaigns Analytics page, so I don't think we need to update this immediately.
~~Build doesn't work (possible incompatibility with the node version I'm using)~~ Should now work with Node v16.11.1

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

dkoo · 2021-09-10T16:10:38Z

Looping in the @Automattic/newspack-product team for additional testing and feedback. I was able to run npm ci && npm run build locally using Node v15.6.0. Confirming that Node v12.* is unable to install dependencies. So this change would require us to update our Node/NPM versions for dev as well as dependencies in all Newspack repos to match. This is not a bad thing and is probably overdue, but I'm not sure what our timeline for being able to do this is.

As for the CI "build" job failing, we probably need to update something in the CircleCI config to use Node v15+. cc @adekbadek as he knows most about our CI config.

dkoo · 2021-09-13T17:42:08Z

@thomasguillot regarding this:

Dependency react-daterange-picker is stale and doesn't appear to be updated anymore. It doesn't list react 17 support, but I suppose it probably supports it anyways.

It seems to me like this package is only used in the DateRangePicker Newspack component, ~~but I'm not seeing this component being used anywhere. Can we deprecate it entirely? Can we replace the third-party package with the core DateTime component instead?~~

Edit: Nevermind, I see it's used in the Campaigns analytics view. Here's another package that's maybe descended from the one we're using that is still maintained: https://github.com/wojtekmaj/react-daterange-picker

I also still think it might be possible to use the core DateTime component using the isDayHighlighted prop.

noahtallen · 2021-09-13T18:47:01Z

I also still think it might be possible to use the core DateTime component using the isDayHighlighted prop.

Yeah, it's always nice to use core components when we can!

noahtallen · 2021-09-14T21:09:53Z

we probably need to update something in the CircleCI config to use Node v15

I just checked, and it looks like the package-lock for the repo was generated with Npm 7, but CircleCI is still on npm 6. It appears to use node 14, which is also working for me locally now.

noahtallen · 2021-09-14T21:52:57Z

I experimented with a few approaches, but haven't found a solution yet.

.circleci/config.yml

adekbadek · 2021-09-16T09:38:55Z

With the build problems resolved (#1142), we now can deal with JS tests & linting failing 🕵️‍♀️

thomasguillot · 2021-09-20T13:03:26Z

@thomasguillot regarding this:

Dependency react-daterange-picker is stale and doesn't appear to be updated anymore. It doesn't list react 17 support, but I suppose it probably supports it anyways.

It seems to me like this package is only used in the DateRangePicker Newspack component, ~~but I'm not seeing this component being used anywhere. Can we deprecate it entirely? Can we replace the third-party package with the core DateTime component instead?~~

Edit: Nevermind, I see it's used in the Campaigns analytics view. Here's another package that's maybe descended from the one we're using that is still maintained: https://github.com/wojtekmaj/react-daterange-picker

I also still think it might be possible to use the core DateTime component using the isDayHighlighted prop.

I leave this up to @adekbadek 😊

noahtallen · 2021-09-23T00:16:33Z

I updated jest and the eslint packages and added a mock for a missing window global. This solved the unit test problem.

The issue with linting is because for some reason, eslint cannot find all of the eslint plugins which @wordpress/eslint-plugin installs. For some reason, it isn't finding them as a transitive dependency of the wp eslint plugin, so I've added all of the plugins it complained about to our dev dependencies. That seems to make eslint happy (other than actual lint issues which need to be fixed)

miguelpeixe · 2021-09-23T19:14:43Z

Although tests are passing, I'm getting build errors with node v16.7.0. We need to update the test images so the node version matches our target version.

According to docker run --rm circleci/php:7.2-node-browsers node -v, the node version for the tests is v14.15.2.

The issue I'm having is node-sass v4.13.0 not being available in the target version v16.7.0. The installed version using ^4.13.0 at this moment is 4.14.1, which supports up to node v14: https://github.com/sass/node-sass/releases/tag/v4.14.1

noahtallen · 2021-09-23T20:20:49Z

Yeah, node-sass has been deprecated -- we had to replace it with plain sass in wp-calypso (see Automattic/wp-calypso#52729)

I was testing with Node 14 locally, which seems to work okay. Which version to we officially support?

miguelpeixe · 2021-09-23T20:41:28Z

Oh, I didn't know that happened! We might have to do the same.

My understanding is that we'd like node to match wp-calypso's version. But I'd like @dkoo and @adekbadek's inputs on this.

dkoo · 2021-09-23T20:44:53Z

Yeah, I think we'll ideally want to match Calypso's version. So that means deprecating node-sass in a similar way for all of our repos. This is one instance where I can see the benefit of a monorepo. 😄

noahtallen · 2021-09-23T22:01:00Z

hahaha, yeah, I've been updating loads of dependencies in calypso, and it seems like every update requires 5 other updates as well 🤦

dkoo · 2021-10-08T16:26:44Z

Hi all, I've been playing with trying to update Node to v16.* and the related dependencies, but I haven't had luck getting a successful build so far. What do we think about an intermediate update to Node v14.* and merging this PR more or less as-is, so we can unblock @noahtallen and the WP.com team?

adekbadek · 2021-11-08T10:25:08Z

Does that only happen on this branch? And/or is it something we can replicate on a temp site for debugging?

@dkoo Yes. DM'd you a temp site

… downgrade react-router-dom

noahtallen · 2021-11-09T20:20:22Z

assets/components/package.json

+	},
+	"dependencies": {
+		"@wordpress/base-styles": "^4.0.0",
+		"@wordpress/components": "^17.0.0",


I think new versions have come out in the meantime (v18 is available), so it might be worth updating again to avoid having to do it later

Like trying to hit a moving target! This PR implements Dependabot, so I might freeze the updates as they are right now since things are currently building and functional.

cool, that also sounds like a good plan!

Like trying to hit a moving target

Yeah, it can be very difficult 😬

dkoo · 2021-11-09T21:04:15Z

Couple of updates: this error is replicable on brand new sites and seems somehow related to webpack and the importing of SCSS files. For example, on the Plugins screen, if you disable importing ./plugins-screen.scss in plugins-screen.js, the error doesn't occur. I'm not seeing any functional errors as a result of it. When mediaelement-and-player.min.js is actually used (e.g. in the classic Widgets screen), the error also doesn't occur. I'm not sure where to go from here on this bug.

Also, when testing Newspack Components as a package imported to another repo (Newspack Blocks), I ran into errors related to CommonJS (module is not defined). Switching the main entrypoint for the package from CJS to ESM as in 3c8aca7 fixes the issue in Newspack Blocks, but I'm not sure of the wider ramifications for Calypso.

dkoo · 2021-11-09T23:33:16Z

Another update! I've found that if I include mediaelement-core as a dependency in our wp_enqueue_script statements, it fixes these JS errors.

But this is frustrating: now I'm getting a new JS error that I haven't seen before when viewing the Campaigns wizard:

Uncaught TypeError: _interopRequireDefault is not a function
    <anonymous> webpack://newspack/./node_modules/dom-helpers/class/addClass.js?:8
    js https://<site_url>/wp-content/plugins/newspack-plugin/dist/popups.js?ver=1636499874:1349
[...]

dkoo · 2021-11-10T21:44:37Z

458beb8 fixes the _interopRequireDefault is not a function build error—which was coming from the recharts package—by replacing recharts with google-react-charts.

The Campaigns wizard should be restored now and all other wizard pages should still be working as expected.

However, I'm now getting failing phpunit tests in CI:

Error: The PHPUnit Polyfills library is a requirement for running the WP test suite.
If you are trying to run plugin/theme integration tests, make sure the PHPUnit Polyfills library (https://github.com/Yoast/PHPUnit-Polyfills) is available and either load the autoload file of this library in your own test bootstrap before calling the WP Core test bootstrap file; or set the absolute path to the PHPUnit Polyfills library in a "WP_TESTS_PHPUNIT_POLYFILLS_PATH" constant to allow the WP Core bootstrap to load the Polyfills.

If you are trying to run the WP Core tests, make sure to set the "WP_RUN_CORE_TESTS" constant to 1 and run `composer install` before running the tests.
Once the dependencies are installed, you can run the tests using the Composer-installed version of PHPUnit or using a PHPUnit phar file, but the dependencies do need to be installed whichever way the tests are run.

I think this is out-of-scope to fix in this PR, though, since it looks like it requires some migration on our part and seems unrelated to the changes in this branch. @Automattic/newspack-product please confirm.

https://make.wordpress.org/core/2021/09/27/changes-to-the-wordpress-core-php-test-suite/

adekbadek

Fixed the PHPUnit thing in 145fd5e

Left one question about newspack-components version, otherwise it works as expected.

assets/components/package.json

.circleci/config.yml

matticbot · 2021-11-16T16:23:34Z

🎉 This PR is included in version 2.0.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot · 2021-11-16T16:48:04Z

🎉 This PR is included in version 2.0.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot · 2021-11-16T16:57:44Z

🎉 This PR is included in version 1.66.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot · 2021-11-18T21:28:30Z

🎉 This PR is included in version 1.66.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

noahtallen self-assigned this Sep 10, 2021

noahtallen requested review from thomasguillot and dkoo September 10, 2021 00:02

dkoo requested a review from a team September 10, 2021 15:37

noahtallen mentioned this pull request Sep 10, 2021

Update to React 17 and latest WordPress packages Automattic/wp-calypso#54793

Merged

30 tasks

noahtallen force-pushed the update/wp-deps branch from c819baa to 2a097d1 Compare September 14, 2021 20:56

adekbadek mentioned this pull request Sep 15, 2021

chore/wp prettier #1142

Merged

6 tasks

noahtallen commented Sep 15, 2021

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

noahtallen added 2 commits September 22, 2021 14:38

Update WordPress dependencies

32cf515

Add node to engines

e244e94

noahtallen force-pushed the update/wp-deps branch from ab4d6f3 to e244e94 Compare September 22, 2021 21:39

noahtallen added 2 commits September 22, 2021 16:48

Update jest

5cc4ad2

Add 'missing' eslint plugins

e180743

noahtallen added 2 commits September 22, 2021 17:43

Resolve all lint errors

dd3be40

Dedupe node modules

19357bf

Merge branch 'master' into update/wp-deps

f411339

chore: use ES modules instead of CommonJS when imported as a package;…

3c8aca7

… downgrade react-router-dom

dkoo mentioned this pull request Nov 9, 2021

chore: update dependencies; slider JS code changes for major version update Automattic/newspack-blocks#904

Merged

6 tasks

noahtallen commented Nov 9, 2021

View reviewed changes

dkoo added 2 commits November 10, 2021 14:23

fix: replace recharts with react-google-charts to fix build issues

458beb8

fix: eslint

f7643f5

fix: console error resulting from mediaelement-core

f7f9b55

dkoo added [Status] Needs Review The issue or pull request needs to be reviewed and removed [Status] Approved The pull request has been reviewed and is ready to merge labels Nov 10, 2021

dkoo and others added 4 commits November 10, 2021 15:36

chore: automatically assign dependabot PRs to Newspack Product group

735e1e7

style: revert whitespace changes to improved diff readability

75f9d60

ci: fix WP version at 5.8.1 until the PHPUnit changes are addressed

145fd5e

https://make.wordpress.org/core/2021/09/27/changes-to-the-wordpress-core-php-test-suite/

test: update popups tests

40b10dc

adekbadek reviewed Nov 11, 2021

View reviewed changes

assets/components/package.json Show resolved Hide resolved

.circleci/config.yml Show resolved Hide resolved

adekbadek approved these changes Nov 11, 2021

View reviewed changes

github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Nov 11, 2021

dkoo merged commit 14e78df into master Nov 11, 2021

dkoo deleted the update/wp-deps branch November 11, 2021 17:08

noahtallen mentioned this pull request Nov 12, 2021

Remove react native dependency Automattic/wp-calypso#57983

Merged

scinos mentioned this pull request Nov 15, 2021

fix(deps): update wordpress monorepo Automattic/wp-calypso#57217

Merged

1 task

matticbot added the released on @alpha label Nov 16, 2021

matticbot added the released label Nov 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update WordPress dependencies #1135

Update WordPress dependencies #1135

noahtallen commented Sep 10, 2021 •

edited by dkoo

Loading

dkoo commented Sep 10, 2021 •

edited

Loading

dkoo commented Sep 13, 2021 •

edited

Loading

noahtallen commented Sep 13, 2021

noahtallen commented Sep 14, 2021

noahtallen commented Sep 14, 2021

adekbadek commented Sep 16, 2021

thomasguillot commented Sep 20, 2021

noahtallen commented Sep 23, 2021 •

edited

Loading

miguelpeixe commented Sep 23, 2021 •

edited

Loading

noahtallen commented Sep 23, 2021

miguelpeixe commented Sep 23, 2021

dkoo commented Sep 23, 2021

noahtallen commented Sep 23, 2021

dkoo commented Oct 8, 2021

adekbadek commented Nov 8, 2021 •

edited

Loading

noahtallen Nov 9, 2021

dkoo Nov 9, 2021

noahtallen Nov 9, 2021

dkoo commented Nov 9, 2021

dkoo commented Nov 9, 2021 •

edited

Loading

dkoo commented Nov 10, 2021

adekbadek left a comment

matticbot commented Nov 16, 2021

matticbot commented Nov 16, 2021

matticbot commented Nov 16, 2021

matticbot commented Nov 18, 2021

Update WordPress dependencies #1135

Update WordPress dependencies #1135

Conversation

noahtallen commented Sep 10, 2021 • edited by dkoo Loading

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Possible issues/follow-up

Other information:

dkoo commented Sep 10, 2021 • edited Loading

dkoo commented Sep 13, 2021 • edited Loading

noahtallen commented Sep 13, 2021

noahtallen commented Sep 14, 2021

noahtallen commented Sep 14, 2021

adekbadek commented Sep 16, 2021

thomasguillot commented Sep 20, 2021

noahtallen commented Sep 23, 2021 • edited Loading

miguelpeixe commented Sep 23, 2021 • edited Loading

noahtallen commented Sep 23, 2021

miguelpeixe commented Sep 23, 2021

dkoo commented Sep 23, 2021

noahtallen commented Sep 23, 2021

dkoo commented Oct 8, 2021

adekbadek commented Nov 8, 2021 • edited Loading

noahtallen Nov 9, 2021

Choose a reason for hiding this comment

dkoo Nov 9, 2021

Choose a reason for hiding this comment

noahtallen Nov 9, 2021

Choose a reason for hiding this comment

dkoo commented Nov 9, 2021

dkoo commented Nov 9, 2021 • edited Loading

dkoo commented Nov 10, 2021

adekbadek left a comment

Choose a reason for hiding this comment

matticbot commented Nov 16, 2021

matticbot commented Nov 16, 2021

matticbot commented Nov 16, 2021

matticbot commented Nov 18, 2021

noahtallen commented Sep 10, 2021 •

edited by dkoo

Loading

dkoo commented Sep 10, 2021 •

edited

Loading

dkoo commented Sep 13, 2021 •

edited

Loading

noahtallen commented Sep 23, 2021 •

edited

Loading

miguelpeixe commented Sep 23, 2021 •

edited

Loading

adekbadek commented Nov 8, 2021 •

edited

Loading

dkoo commented Nov 9, 2021 •

edited

Loading