[ETK] Update `@wordpress/interface` to the latest published version #73629

fullofcaffeine · 2023-02-22T01:23:17Z

Update it to match the changes introduced by the same package in Gutenberg 15.2.

IMPORTANT: This PR should be merged before releasing Gutenberg 15.2 to WPCOM.

The mismatch between versions is causing a WSOD in simple sites (and probably in AT, too). Gutenberg 15.2 introduced some changes in this package (WordPress/gutenberg#47498), but ETK ends up loading an older version without these changes. The ETK version is probably loaded (enqueued) after the GB version and then replaces most of it*, while the GB version still tries to import from the replaced module selectors.js and fails, since it contains the old code.

*only some parts, probably because the package is code-split and ETK only loads some modules (like selectors.js)? I haven't looked deeply, this is just an assumption.

Related to WordPress/gutenberg#47498.

Slack discussion: p1677020860615489-slack-CBTN58FTJ (also a good primer on why the .yarmlrc.yml changes were needed)

Sum up on Slack: p1677778450155879/1677777680.547219-slack-CBTN58FTJ

Proposed Changes

Update the version of @wordpress/interface in ETK to match the one in Gutenberg 15.2.

Testing Instructions

Apply this diff: D102317-code
to your sandbox, make sure your test sandboxed site has the gutenberg-edge sticker applied;
Try to create a new post
You'll get a WSOD
Sync a custom ETK build from this branch (yarn && yarn dev --sync)
Reload the page
It should load the editor without any fatal errors.

github-actions · 2023-02-22T01:23:30Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-58410

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-58410&env=jetpack

matticbot · 2023-02-22T01:24:02Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

matticbot · 2023-02-22T01:24:19Z

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit update/etk-wordpress-interface-to-latest-version on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

fullofcaffeine · 2023-02-22T23:25:00Z

client/blocks/time-mismatch-warning/test/index.jsx

 describe( 'TimeMismatchWarning', () => {
 	beforeAll( () => {
-		jest.spyOn( global, 'Date' ).mockImplementation( () => ( { getTimezoneOffset: () => 240 } ) );
+		global.Date = MockedDate;


These tests were failing with:

TypeError: Date.now is not a function at ../node_modules/requestidlecallback/index.js:57:21 at MutationObserver.onInputorMutation (../node_modules/requestidlecallback/index.js:87:3)

I haven't dug too deep, but looks like this is caused by the requestidlecallback package, which now uses Date.now . Before the package updates that are part of this PR, Date.now was not in any of the code paths for this test (apparently), so something changed after the update.

With the mockedImplementation approach, it discarded Date's original implementation to only define the custom getTimezoneOffset. I've tried to use spyOn + mockImplementation to return a custom class based off Date but failed - it was much simpler to just override the global variable and re-assign the original object to it in the afterAll callback.

I wonder why this test triggered a portion of code that uses this package now while it didn't before the update. Maybe that package was already used, and it was recently updated to use Date.now. Provided tests pass, I think it's okay to consider this relatively safe to ship, but if you have more insights, let me know. Besides, in the case of globals like Date, I think it's safer to keep the original implementation accessible and mock the functions over it. The previous approach based on mockImplementation removed all other functions and properties (including the static now).

No concerns from my side about this change. We've seen similar issues in Gutenberg tests too. It's usually that something tries to access Date before the jsdom mock has been setup, or something inadvertently overrides it for the test.

To match the changes introduced by the same package in Gutenberg 15.2.

…ontinue working with React 17

arthur791004 · 2023-02-23T08:28:03Z

Referring to #73078 (comment), the @wordpress/* packages have their interconnected dependencies, I'm not sure we can only upgrade @wordpress/interface package. For example, there are lots of version changes in the yarn.lock, it might bring up unexpected side effects to Calypso

FYI @Automattic/team-calypso-frameworks is working on Try: Upgrade @wordpress packages to pre-React 18

tyxla · 2023-02-23T09:16:55Z

@arthur791004 the big difference between this PR and #73078 is that this is updating the @wordpress/interface dependency only in ETK, and not for the rest of Calypso monorepo.

I've taken a look around the yarn.lock changes in this PR and I don't have any concerns. There are several additional changes caused by dependency deduplication, but they are fine because they resolve to versions of the packages that still use React 17. The only versions that use React 18 are packages that are specifically only used by ETK, so I don't see how the rest of Calypso could be affected.

arthur791004 · 2023-02-23T09:25:08Z

Got it! Looks like I misunderstood some changes in yarn.lock 😅

tyxla

I've taken a more extensive look over the yarn.lock updates and have some concerns - we this package upgrade, and specifically the deduplication might have unexpected effects on the rest of Calypso. I'm not sure we intended to upgrade all those @wordpress packages for the test of the monorepo.

tyxla · 2023-02-23T09:17:27Z

package.json

@@ -300,7 +300,9 @@
 		"newspack-components/@wordpress/element": "4.7.0",
 		"newspack-components/@wordpress/i18n": "4.9.0",
 		"keytar@npm:7.7.0/node-addon-api": "3.1.0",
-		"lzma-native": "^8.0.5"
+		"lzma-native": "^8.0.5",
+		"@wordpress/primitives/@wordpress/element": "4.20.0",


Should we pin this to a particular version of @wordpress/primitives to avoid this propagating to another version of @wordpress/primitives and accidentally into the rest of the monorepo?

Good point 👍🏻

Hmm, after resetting the yarn.lock to the trunk version and re-installing the new version of @wordpress/interface in ETK without deduping, these resolution overrides don't seem to be needed anymore, the storybook task is working fine.

I removed those entries.

tyxla · 2023-02-23T09:23:32Z

client/blocks/time-mismatch-warning/test/index.jsx

 describe( 'TimeMismatchWarning', () => {
 	beforeAll( () => {
-		jest.spyOn( global, 'Date' ).mockImplementation( () => ( { getTimezoneOffset: () => 240 } ) );
+		global.Date = MockedDate;


No concerns from my side about this change. We've seen similar issues in Gutenberg tests too. It's usually that something tries to access Date before the jsdom mock has been setup, or something inadvertently overrides it for the test.

tyxla · 2023-02-23T09:28:28Z

yarn.lock

+"@types/react-dom@npm:*, @types/react-dom@npm:>=16.9.0, @types/react-dom@npm:^18.0.6":
+  version: 18.0.11
+  resolution: "@types/react-dom@npm:18.0.11"


This is where things could eventually get thorny, but if there are no TS errors introduced, we're likely fine for now. We should keep an eye on this. Ideally, we should upgrade to the newest @wordpress packages and upgrade the repo to use React 18 as soon as possible.

tyxla · 2023-02-23T09:29:28Z

yarn.lock

+"@wordpress/a11y@npm:^3.10.0, @wordpress/a11y@npm:^3.13.0, @wordpress/a11y@npm:^3.27.0, @wordpress/a11y@npm:^3.9.0":
+  version: 3.27.0
+  resolution: "@wordpress/a11y@npm:3.27.0"


Seems like this is forcing the rest of Calypso to use @wordpress/a11y@3.27.0 - is it intentional? I think we should refrain from affecting the rest of the monorepo.

tyxla · 2023-02-23T09:29:47Z

yarn.lock

+  languageName: node
+  linkType: hard
+
+"@wordpress/date@npm:^4.13.0, @wordpress/date@npm:^4.27.0, @wordpress/date@npm:^4.9.0":
