[DO NOT MERGE] Test @wordpress/*@next packages

[manzoorwanijk]

Testing the @wordpress/* packages as requested in WordPress/gutenberg#73822 (comment)

Proposed changes:

Other information:

• Have you written new tests for your changes, if applicable?
• Have you checked the E2E test CI results, and verified that your changes do not break them?
• Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

• Go to '..'

[jsnajdr]

I would aim for getting rid of all the patches we have for @wordpress/* packages. 🙂

I fixed the questionable imports in WordPress/gutenberg#74530 and published a new next version, with the 06ee73755 hash.

Then I updated this PR branch to use that version, removed the patches from pnpm-workspace.yml.

I also had to patch a sass dependency in .pnpmfile.cjs because when @manzoorwanijk updated Gutenberg sass-embedded to its latest version in WordPress/gutenberg#74434, it dragged in a new optional dependency that depends on sass. pnpm install was getting aborted.

But now I'm facing issues that I don't know how to resolve. The lockfile seems to be broken. There are both 06ee73755 and old ba3aee3a2 versions of packages and I'm unable to deduplicate them.

For example, the pnpm-lock.yml contains several records for @wordpress/[email protected], with different peer dependencies. And it's @wordpress/warning dependency is always the old ba3aee3a2 one, although the edit-post package.json specifies the new one and the new one exists. I don't understand why pnpm keeps insisting on the old one.

[anomiex]

This is fine for this DNM PR, but once this comes up for real I'd recommend instead excluding the deps on these two packages entirely instead of neutering them like this.

[anomiex]

Handled it in #46552.

[anomiex]

There are both 06ee73755 and old ba3aee3a2 versions of packages and I'm unable to deduplicate them.

Looks like part of the problem there is that a version like 1.2.3-next.ba3aee3a2.0 compares as newer than 1.2.3-next.06ee73755.0 (because ba3aee3a2 > 06ee73755 as a string comparison), so it tends to incorrectly prefer the former for a caret version.

[anomiex]

I fixed the questionable imports in WordPress/gutenberg#74530 and published a new next version, with the 06ee73755 hash.

Looks like you missed one: https://github.com/WordPress/gutenberg/pull/74530/changes#r2683062457 🙂

[jsnajdr]

version like 1.2.3-next.ba3aee3a2.0 compares as newer than 1.2.3-next.06ee73755.0 (because ba3aee3a2 > 06ee73755 as a string comparison)

Well that's bad, and it's a bug in Gutenberg's version naming scheme for next packages. Let me see what we can do about it.

Looks like you missed one:

The moment-timezone omission was intentional, as it's the consumer (Jetpack) who should define an alias to a more optimized version.

On a second thought, I should probably have resolved this also to moment-timezone.js, and let consumers specify the alias for the name including the .js suffix.

[anomiex]

Well that's bad, and it's a bug in Gutenberg's version naming scheme for next packages. Let me see what we can do about it.

One straightforward fix would be to name it like 1.2.3-next.$timestamp.$sha.0. Or, if these are always created via a build system and the build system has a monotonic build ID of some sort, that could work too.

The moment-timezone omission was intentional, as it's the consumer (Jetpack) who should define an alias to a more optimized version.

I don't see that documented in the package?

On a second thought, I should probably have resolved this also to moment-timezone.js, and let consumers specify the alias for the name including the .js suffix.

👍

[jsnajdr]

One straightforward fix would be to name it like 1.2.3-next.$timestamp.$sha.0.

I'm trying to fix this in WordPress/gutenberg#74589, with a slightly different approach where the version number is 1.2.3-next.0+$sha. The .0 suffix added by Lerna should provide the auto-incrementing prerelease version number. And the commit SHA is only after the +, not affecting the semver comparison between two versions.

[anomiex]

That sounds like it should work. 👍

[anomiex]

Well, going forward anyway. 1.2.3-next.1 will be counted as being before 1.2.3-next.somesha by semver rules ("Numeric identifiers always have lower precedence than non-numeric identifiers.").

[jsnajdr]

Well, going forward anyway. 1.2.3-next.1 will be counted as being before 1.2.3-next.somesha by semver rules

That's why I in the end merged a version that uses the 1.2.3-next.v.1 format. The v bit ensures that the version is always considered newer than any possible somesha one. And we can remove the v after the next public release that bumps the main 1.2.3 part of the version.

[jsnajdr]

@anomiex @manzoorwanijk I updated this branch to the latest .next.v.0 packages and the results are very good: just one e2e failure for Jetpack sync. Can you determine if that's just a flaky test or is something still broken?

[manzoorwanijk]

Yeah, the results are great. @anomiex Should we also see if we can clean up the .pnpmfile.cjs file?

[anomiex]

I removed one unnecessary item from pnpmfile. For the real upgrade, we'll want most of what's left. We'd leave out the change to "next" on line 220. The extra .replace() on line 103 also isn't necessary, but won't hurt anything.

We'll also need the fix in projects/plugins/jetpack/tools/webpack.config.extensions.js, and the react deps in tools/js-tools/package.json thanks to @wordpress/stylelint-config adding a dep on @wordpress/theme.

[manzoorwanijk]

Following WordPress/gutenberg#74576, we should be able to completely get rid of this file.

jetpack/projects/js-packages/ai-client/src/api-fetch/index.ts

Lines 4 to 14 in 140c7bb

	import apiFetchMod from '@wordpress/api-fetch';
	/**
	* Types
	*/
	import type { APIFetchOptions } from '@wordpress/api-fetch';
	// @wordpress/api-fetch (as of 6.47.0) declares itself in such a way that tsc and node see the function at apiFetchMod.default
	// while some other environments (including code running inside WordPress itself) see it at apiFetch.
	// See https://arethetypeswrong.github.io/?p=@wordpress/[email protected]
	// This is a helper to simplify the usage of the api-fetch module on the ai-client package.
	const apiFetch = 'default' in apiFetchMod ? apiFetchMod.default : apiFetchMod;

[jsnajdr]

we should be able to completely get rid of this file.

The caveat is that you never use the default export of api-fetch, only the new named one. The bug with the default export is not yet fixed. To fix that, we'll need to start providing dual .d.ts files. Instead of the current build-types/index.d.ts, which is always interpreted as CJS, we'll need to have also build-types/index.d.mjs.

[manzoorwanijk]

The caveat is that you never use the default export of api-fetch, only the new named one.

Yes, that is what I meant. Using named import can help us get rid of that file.

[anomiex]

Following WordPress/gutenberg#74576, we should be able to completely get rid of this file.

I think we'll need to wait for a few WordPress releases though, until that change is in the Core-bundled copy of the lowest version we support.