Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP wp-now: update blueprints dependencies #116

Closed
wants to merge 5 commits into from

Conversation

sejas
Copy link
Collaborator

@sejas sejas commented Oct 4, 2023

This PR is a draft WIP.

What?

Updates Playground dependencies

Why?

@wp-playground/blueprints v0.2.0 removes the dependency on DOMParser for the activatePlugin blueprint step.

Testing Instructions

  • Run nvm use
  • npm install
  • npx nx build wp-now
  • Create the blue print file b.json
  • Run node dist/packages/wp-now/cli.js start --blueprint=b.json
  • Observe no errors in the console and the WordPress page opens successfully with the plugin notification installed.

b.json

{
	"$schema": "https://playground.wordpress.net/blueprint-schema.json",
	"landingPage": "/wp-admin/",
	"preferredVersions": {
		"php": "8.0",
		"wp": "latest"
	},
	"steps": [
		{
			"step": "login",
			"username": "admin",
			"password": "password"
		},
		{
			"step": "installPlugin",
			"pluginZipFile": {
				"resource": "wordpress.org/plugins",
				"slug": "notification"
			},
			"options": {
				"activate": true
			}
		}
	]
}

@sejas sejas added the wp-now label Oct 4, 2023
@sejas sejas self-assigned this Oct 4, 2023
@sejas
Copy link
Collaborator Author

sejas commented Oct 4, 2023

I'm running into a couple of errors that we may need to fix on the dependencies.

Error 1

The new package needs the File class, therefore node v20 even if we don't sue blueprints.

node dist/packages/wp-now/cli.js start                  
file:///playground-tools/node_modules/@wp-playground/blueprints/index.js:61
class Oi extends File {
                 ^

ReferenceError: File is not defined
    at file:///playground-tools/node_modules/@wp-playground/blueprints/index.js:61:18
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

Node.js v18.13.0

Error 2

With Node v20, the activatePlugin step fails to move the plugin to VFS. I'll find a solution in the next few days.

node dist/packages/wp-now/cli.js start --blueprint=b.json
Starting the server......
directory: /playground-tools
mode: playground
php: 8.0
wp: latest
WordPress latest folder already exists. Skipping download.
SQLite folder already exists. Skipping download.
blueprint steps: 2
Blueprint step completed: login
Proceeding without the Notification plugin. Could not install it in wp-admin. The original error was: Error: Could not move "/tmp/assets/Notification/notification": Cross-device link.
Error: Could not move "/tmp/assets/Notification/notification": Cross-device link.
    at descriptor.value (/playground-tools/node_modules/@php-wasm/node/index.cjs:67481:17)
    at Cs (file:///playground-tools/node_modules/@wp-playground/blueprints/index.js:398:20)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Module.Ji [as installPlugin] (file:///playground-tools/node_modules/@wp-playground/blueprints/index.js:410:36)
    at async a (file:///playground-tools/node_modules/@wp-playground/blueprints/index.js:8090:30)
    at async Object.run (file:///playground-tools/node_modules/@wp-playground/blueprints/index.js:8035:21)
    at async zu (file:///playground-tools/node_modules/@wp-playground/blueprints/index.js:8125:3)
    at async startWPNow (file:///playground-tools/dist/packages/wp-now/main.js:609:5)
    at async startServer (file:///playground-tools/dist/packages/wp-now/main.js:878:42)
    at async Object.handler (file:///playground-tools/dist/packages/wp-now/main.js:1050:25) {
  [cause]: ErrnoError
      at Object.ensureErrnoError (/playground-tools/node_modules/@php-wasm/node/index.cjs:20271:33)
      at Object.staticInit (/playground-tools/node_modules/@php-wasm/node/index.cjs:20279:10)
      at Object.init3 (/playground-tools/node_modules/@php-wasm/node/index.cjs:24127:6)
      at loadPHPRuntime (/playground-tools/node_modules/@php-wasm/node/index.cjs:67495:38)
      at doLoad (/playground-tools/node_modules/@php-wasm/node/index.cjs:68407:31)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async _NodePHP.load (/playground-tools/node_modules/@php-wasm/node/index.cjs:68379:12)
      at async startWPNow (file:///playground-tools/dist/packages/wp-now/main.js:546:7)
      at async startServer (file:///playground-tools/dist/packages/wp-now/main.js:878:42)
      at async Object.handler (file:///playground-tools/dist/packages/wp-now/main.js:1050:25) {
    node: undefined,
    setErrno: [Function (anonymous)],
    errno: 75,
    message: 'FS error'
  }
}
Blueprint step completed: installPlugin
Server running at http://localhost:8881

@adamziel
Copy link
Collaborator

adamziel commented Oct 6, 2023

@sejas for the File class you'll need a polyfill.

The cross-device link error is weird, though. I noticed your disk path starts with /playground-tools. Are you working in a Docker or a VM? Does this error happen to you with the older versions of the Blueprint package, too? Maybe it's unrelated to Blueprints and related to that specific dev env setup?

@adamziel adamziel marked this pull request as ready for review October 6, 2023 16:41
@adamziel
Copy link
Collaborator

adamziel commented Oct 6, 2023

I clicked "ready for review" to see the CI tests.

@sejas
Copy link
Collaborator Author

sejas commented Oct 6, 2023

@sejas for the File class you'll need a polyfill.

What's unexpected for me, is that the class File is loaded even if I don't run any blueprint. I'll investigate it further.
I agree that polyfills will be needed.

The cross-device link error is weird, though. I noticed your disk path starts with /playground-tools. Are you working in a Docker or a VM? Does this error happen to you with the older versions of the Blueprint package, too? Maybe it's unrelated to Blueprints and related to that specific dev env setup?

Sorry for the confusion, I shared a modified "error logs" to remove my home directory. It's a normal Mac environment.
Debugging a bit, I've found the line that raises the error and the values.

https://github.com/WordPress/wordpress-playground/blob/32f17e1216030dc35412a049958d57e1e67d970a/packages/playground/blueprints/src/lib/steps/install-asset.ts#L86

playground.mv( '/tmp/assets/Notification/notification', '/var/www/html/wp-content/plugins/notification' )

I think it's a general limitation on php-wasm/node moving files from VFS

I tried unzipping directly in the folder, and the files appeared in the folder, although it had the extra folder produced by the unzipping. But it worked.

@adamziel
Copy link
Collaborator

adamziel commented Oct 12, 2023

So sounds like it's just about the File class polyfill then? The Blueprint JS library is imported whether you apply a Blueprint or not which is likely why that errors always happens.

@eliot-akira
Copy link
Contributor

The new package needs the File class, therefore node v20 even if we don't use blueprints.

For Node.js 18 support, here are some candidates to polyfill the File class.

  • NPM package web-file-polyfill

    Its GibHub repo was archived 2 years ago. The File class implementation looks simple (here) but it depends on a polyfill for Blob (web-blob) whose repo no longer exists.

  • File API polyfill - Isomorphic File, FileReader, and FileReaderSync for Node.js, Deno, Bun, and browsers

    This is a more recent library, @webfill/fileapi.


Here's a link to a discussion about removing the use of DOMParser from blueprints, so the steps can run on server side.

..That was done for activatePlugin and activateTheme already, and I worked on a pull request for rewriting installPlugin and installTheme.

The remaining step that's yet to be rewritten is importFile.

..This workaround requires that all Blueprint steps that use the File class must import it from steps/common, instead of assuming that it exists as a global name. That may be convenient for polyfilling the same class for Node.js 18.

@adamziel
Copy link
Collaborator

adamziel commented Dec 19, 2023

@sejas WordPress/wordpress-playground#875 fixed the Node.js v18 issue! 🎉 You can update the Playground packages to version 0.5.1 and confirm.

The other one with cross-device move is still a problem, though. WordPress/wordpress-playground#846 may solve it once its ready.

adamziel added a commit to WordPress/wordpress-playground that referenced this pull request Dec 20, 2023
…root (#886)

Solves the [error discovered by
@sejas](WordPress/playground-tools#116 (comment))
caused by calling `php.mv(fromPath, toPath)` when `fromPath` is a
mounted local directory and `toPath` is a MEMFS directory. Emscripten
doesn't support such a scenario:

```
Proceeding without the Notification plugin. Could not install it in wp-admin. The original error was: Error: Could not move "/tmp/assets/Notification/notification": Cross-device link.
Error: Could not move "/tmp/assets/Notification/notification": Cross-device link.
    at descriptor.value (/playground-tools/node_modules/@php-wasm/node/index.cjs:67481:17)
```

The solution proposed in this PR replaces a `/tmp` directory with a
randomly-named temporary directory inside `wp-content`. `/tmp` doesn't
necessarily point to a system temp directory and needs to be revisited
anyway. Whether we should use a temporary directory inside `wp-content`
is another matter, but that part may be revisited once the [recursive
cp](#846) feature
is added.
@adamziel
Copy link
Collaborator

WordPress/wordpress-playground#886 solved the cross-device move problem. I went ahead and updated this PR to use the new 0.5.2 version of the Playground packages:

CleanShot 2023-12-20 at 10 19 51@2x

cc @sejas @danielbachhuber

@sejas
Copy link
Collaborator Author

sejas commented Feb 5, 2024

Closing this PR since the Polyfills were introduced on WordPress/wordpress-playground#875 and we are already using that version of the playground.

I'm still seeing an error related to crypto.randomUUID. I'll comment on the issue.

@sejas sejas closed this Feb 5, 2024
@sejas sejas deleted the update/wp-now-blueprints-dependency branch February 5, 2024 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants