-
Notifications
You must be signed in to change notification settings - Fork 42
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
Conversation
…date/wp-now-blueprints-dependency
I'm running into a couple of errors that we may need to fix on the dependencies. Error 1The new package needs the
Error 2With Node v20, the
|
@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 |
I clicked "ready for review" to see the CI tests. |
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.
Sorry for the confusion, I shared a modified "error logs" to remove my home directory. It's a normal Mac environment. 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. |
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. |
For Node.js 18 support, here are some candidates to polyfill the
Here's a link to a discussion about removing the use of
|
@sejas WordPress/wordpress-playground#875 fixed the Node.js v18 issue! 🎉 You can update the Playground packages to version The other one with cross-device move is still a problem, though. WordPress/wordpress-playground#846 may solve it once its ready. |
…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.
WordPress/wordpress-playground#886 solved the cross-device move problem. I went ahead and updated this PR to use the new |
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 |
This PR is a draft WIP.
What?
Updates Playground dependencies
Why?
@wp-playground/blueprints
v0.2.0 removes the dependency onDOMParser
for theactivatePlugin
blueprint step.Testing Instructions
nvm use
npm install
npx nx build wp-now
b.json
node dist/packages/wp-now/cli.js start --blueprint=b.json
notification
installed.b.json