👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @lithrel! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

The unit test error is unfortunate:

SyntaxError: Cannot use import statement outside a module

I think this is because wp-env is still using CJS and not ESM, but this 3rd party dependency (yaml) might not provide a CJS option.

I don't know if there's a decent workaround -- we might have to explore migrating wp-env to ESM (which would be nice to do eventually anyways, but I don't know if there are blockers to that in the Gutenberg repo)

/home/runner/work/gutenberg/gutenberg/packages/env/node_modules/yaml/browser/index.js:3
    import * as YAML from './dist/index.js'

I think this is because wp-env is still using CJS and not ESM, but this 3rd party dependency (yaml) might not provide a CJS option.

It's weird. It does have a CJS build under dist/index.js, see https://unpkg.com/browse/yaml@2.3.1/package.json. Shouldn't Node pick that export?

Shouldn't Node pick that export?

Hm, I'd think so! 🤔 Maybe an issue since we're still using node 14? (#52363 )

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

• Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
• Labels found: First-time Contributor, [Package] Env.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

Maybe an issue since we're still using node 14? (#52363 )

I just rebased to get Node 16, and same error apparently 🤔
I can get those tests to pass by adding in Jest config

"transformIgnorePatterns": [
    	"/node_modules/(?!(docker-compose|yaml)/)",
	"\\.pnp\\.[^\\/]+$"
],

but I feel like I'm missing something more obvious

I'd recommend discarding all package-lock changes and then run npm install again! I wonder if we could actually write wp-env with ESM now because nothing imports its code

I think transformIgnorePatterns is a fine workaround -- in this case, it does look like they need to be transpiled for Jest to use them. If those packages don't provide CJS output, and we don't use ESM, we won't be compatible. Normally, packages provide both types for compatibility, but I guess not in this case

@noahtallen I tested removing package-lock and reinstalling, but still the same issue. So I implementend the transformIgnorePatterns workaround (and also changed a line in documentation), it's passing now 👍

If you have a ticket for converting this package to ESM I can take a look in the following weeks 👀

It looks like the resolution problem is because of the conditional exports in yaml being incompatible with Jest. As far as I can tell, the standard is for "import" and "require" to be the conditional exports but yaml uses "node" and "default" instead. This ends up being resolved incorrectly and it tries to load the browser code as default. I'm going to submit an upstream fix to "yaml " and then once that gets released an upstream fix to "docker-compose".

While we wait for that though, I think it would be worthwhile to get this PR across the finish line. docker-compose has been unsupported for quite some time and this change is likely fine as long as we mark it as major. Before we merge though, we should update docker-compose again since it has been a long time 😄

Before we merge though, we should update docker-compose again since it has been a long time 😄

@ObliviousHarmony Rebased and updated docker-compose to 0.24.3 👍

Done @ObliviousHarmony :)

Run all modified commands:

• wp-env start
• wp-env logs
• wp-env run cli wp user list
• wp-env stop
• wp-env clean
• wp-env destroy

I would like to inform you that all of these commands worked correctly on Windows 11 🎉

Looks like we just need to fix the changelog conflict, and then I'm happy to merge!

@noahtallen rebased/resolved 👍

👋 FWIW, npm run wp-env start has recently started failing for me; I believe it was correlated with this change. The error message is as follows:

> gutenberg@17.3.0-rc.1 wp-env
> wp-env start

✖ Error while running docker compose command.
unknown shorthand flag: 'f' in -f
See 'docker --help'.

Usage:  docker [OPTIONS] COMMAND

The reason seems to be that docker-compose is now a Docker plugin (rather than a separate script or executable) that wasn't installed on my system (and hence docker compose -f ... was failing).

I’m using colima on my system, and have installed it — and Docker — via brew. I found the solution to my issue here:

Compose is now a Docker plugin. For Docker to find this plugin, symlink it:
    mkdir -p ~/.docker/cli-plugins
    ln -sfn $HOMEBREW_PREFIX/opt/docker-compose/bin/docker-compose ~/.docker/cli-plugins/docker-compose

After running those commands, npm run wp-env start is working again 🎉

Just sharing in case anyone is experiencing the same issue.

Very nice! I have Docker installed with Docker Desktop, and it seemed to work out of the box with that setup.

FYI this did not properly update the version of docker-compose in the lock file, which caused some issues later on as a new version of docker-compose was released with a breaking change.

See #59183, #6133