wp now: load virtually config constants to allow running multiple projects at the same time #343

sejas · 2023-05-15T19:57:19Z

Closes Local Environment: Unable to run two projects at the same time #324

What?

New blueprint function to autoload virtually WP config contains. It doesn't write or update user files, it only creates a file in VFS and use php.ini auto_prepend_file
It allows to run multiple wp-now projects at the same time
It includes tests.

Why?

The current blueprint defineSiteUrl creates a json file that is re-used in every wp-now instance. Creating conflicts between site redirections. See #324

How?

Created defineVirtualWpConfigConsts an alternative of defineWpConfigConsts.

Testing Instructions

Check out this branch.
Prepare two directories with a theme or plugin
run npx nx preview wp-now start --path=/path/to/theme-plugin-1
Observe it the WordPress environment works as expected
run npx nx preview wp-now start --path=/path/to/theme-plugin-2
Observe the new instance runs as expected
Observe the first instance is still working in a different port without any URL redirection.
Observe the wp-config.php was not modified and you don't have any new playground-consts.json.json file in the root of your folder. (For a clean test, remove your files inside ~/.wp-now)

…_file to define php constants

…p-config.php

…tant instead of given path

…to update/wp-now-load-virtually-config-constants

danielbachhuber

Works as expected 👍

I was able to run wp-now start in a gutenberg plugin directory and wp-now start in a twentytwenty theme directory. Both sites worked fine side by side.

wp-config.php still seems to define constants, though:

$ cat ~/.wp-now/wordpress-versions/latest/wp-config.php
<?php

// ** Database settings - You can get this info from your web host ** //
/** The name of the database for WordPress */
define( 'DB_NAME', 'database_name_here' );

Would it make sense to remove some of the constants to prevent them from being defined twice?

Also (outside the scope of this PR): how should we let users define their own wp-config.php constants?

packages/playground/blueprints/src/lib/steps/define-virtual-wp-config-consts.ts

adamziel · 2023-05-15T20:34:56Z

I like auto_prepend_file! The only issue I have is this step isn't composable. If I use it twice, I'll override all previously defined constants. The existing step uses JSON so that the second call can load all the existing constants, append new ones, and save everything at once. Instead of introducing many steps accomplishing the same task, this PR could refactor the existing JSON-based one to use auto_prepend_file instead of modifying wp-config.php.

wojtekn · 2023-05-16T07:07:43Z

packages/wp-now/src/wp-now.ts

-	defineWpConfigConsts,
-	login,
-} from '@wp-playground/blueprints';
+import { defineVirtualWpConfigConsts, login } from '@wp-playground/blueprints';


Do we still need defineWpConfigConsts, or can we remove the old one?

@wojtekn , I was not sure if it's used by other apps that import this library.

wojtekn · 2023-05-16T07:18:17Z

wp-config.php still seems to define constants, though:

$ cat ~/.wp-now/wordpress-versions/latest/wp-config.php
<?php

// ** Database settings - You can get this info from your web host ** //
/** The name of the database for WordPress */
define( 'DB_NAME', 'database_name_here' );

I would leave this as expected behavior - that way, wp-now won't modify the developer's environment at all and will define the constants it needs to run.

Would it make sense to remove some of the constants to prevent them from being defined twice?

WP_HOME and WP_SITEURL are not defined in wp-config-sample.php, so we should be good.

If we are worried about someone breaking their wp-now local environment, we can document that in the readme and say that for smooth work with wp-now, user shouldn't have WP_HOME and WP_SITEURL defined in their config.php file.

Also (outside the scope of this PR): how should we let users define their own wp-config.php constants?

We are not replacing the wp-config.php behavior at all, so the user can still use it to modify any default constant or define any other constant.

In the future, if we add support for the .wp-now.json file, we could also let users define their own constants there, so they would be brought for PHP by defineVirtualWpConfigConsts step.

wojtekn · 2023-05-16T07:24:28Z

I like auto_prepend_file! The only issue I have is this step isn't composable. If I use it twice, I'll override all previously defined constants.

@adamziel I think it's the intention as we are defining run-time constants here, not constants that are part of the user's configuration. If users want to define their own constants and have them preserved, they can use wp-config.php which is a default way of doing that.

Also, if we decide to add support for environment-specific configuration files like .wp-now.json, we can also add support for defining additional constants there.

The existing step uses JSON so that the second call can load all the existing constants, append new ones, and save everything at once. Instead of introducing many steps accomplishing the same task, this PR could refactor the existing JSON-based one to use auto_prepend_file instead of modifying wp-config.php.

Could we just remove the previous step?

wojtekn

Looks great and works as expected.

katinthehatsite

It worked as expected for me. I tried with Gutenberg and Twenty Twenty One theme and both projects launched well.

One thing that I came across is that the server failed to start because it could not read /var/www/html/wp-config-sample.php. I also got a similar error with Sqlite plugin (I will paste the error below). I had to delete both wordpress-versions and sqlite-database-integration-main folders in ~/.wp-now/ directory before I could run both of these projects at the same time. I understand that this perhaps is not related directly to this issue but should we create a different issue to prevent this from happening?

This was an error:

Starting the server......
directory: /Users/katerynakodonenko/Documents/A8C-Projects/twentytwentyone
mode: theme
php: 8.0
wp: latest
WordPress latest folder already exists. Skipping download.
Sqlite folder already exists. Skipping download.
Error: Could not read "/var/www/html/wp-config-sample.php": There is no such file or directory OR the parent directory does not exist.
    at _NodePHP.descriptor.value (/Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/php-wasm/node/index.cjs:67319:17)
    at _NodePHP.readFileAsText (/Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/php-wasm/node/index.cjs:67760:42)
    at _NodePHP.descriptor.value (/Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/php-wasm/node/index.cjs:67312:23)
    at initWordPress (file:///Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/wp-now/main.js:464:9)
    at runPluginOrThemeMode (file:///Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/wp-now/main.js:445:9)
    at startWPNow (file:///Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/wp-now/main.js:382:13)
    at async startServer (file:///Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/wp-now/main.js:562:42)
    at async Object.handler (file:///Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/wp-now/main.js:668:25) {
  [cause]: ErrnoError
      at Object.ensureErrnoError (/Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/php-wasm/node/index.cjs:20233:33)
      at Object.staticInit (/Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/php-wasm/node/index.cjs:20241:10)
      at Object.init3 (/Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/php-wasm/node/index.cjs:24083:6)
      at loadPHPRuntime (/Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/php-wasm/node/index.cjs:67338:38)
      at doLoad (/Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/php-wasm/node/index.cjs:68228:31)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async Function.load (/Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/php-wasm/node/index.cjs:68200:12)
      at async startWPNow (file:///Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/wp-now/main.js:339:15)
      at async startServer (file:///Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/wp-now/main.js:562:42)
      at async Object.handler (file:///Users/katerynakodonenko/Documents/A8C-Projects/wordpress-playground/dist/packages/wp-now/main.js:668:25) {
    node: undefined,
    setErrno: [Function (anonymous)],
    errno: 44,
    message: 'FS error'
  }
}
Failed to start the server: Could not read "/var/www/html/wp-config-sample.php": There is no such file or directory OR the parent directory does not exist.

sejas · 2023-05-16T09:27:25Z

I like auto_prepend_file! The only issue I have is this step isn't composable. If I use it twice, I'll override all previously defined constants. The existing step uses JSON so that the second call can load all the existing constants, append new ones, and save everything at once. Instead of introducing many steps accomplishing the same task, this PR could refactor the existing JSON-based one to use auto_prepend_file instead of modifying wp-config.php.

@adamziel ,
I was thinking about this, and I had different options in mind:

Option 1. `setPhpIniEntry` is an independent step.

Move setPhpIniEntry to a different step.
So the user could edit VFS_CONFIG_FILE_PATH manually or other steps could append data that file.
And then, call the step setPhpIniEntry.

export const defineVirtualWpConfigConsts: StepHandler<DefineVirtualWpConfigConstsStep> = 
async (playground, { consts }) => {
	playground.mkdir(VFS_CONFIG_PATH);
	playground.writeFile(VFS_CONFIG_FILE_PATH, buildConfigFileContents(consts));
-	playground.setPhpIniEntry('auto_prepend_file', VFS_CONFIG_FILE_PATH);
};

Option 2. `defineVirtualWpConfigConsts` accepts a string

export const defineVirtualWpConfigConsts: StepHandler<	DefineVirtualWpConfigConstsStep> =
async (playground, {
            consts,
+         phpCodeExtra
         }) => {
	playground.mkdir(VFS_CONFIG_PATH);
+	playground.writeFile(VFS_CONFIG_FILE_PATH, buildConfigFileContents(consts) + phpCodeExtra);
	playground.setPhpIniEntry('auto_prepend_file', VFS_CONFIG_FILE_PATH);
};

I like more option 1, and we open the possibility to add more steps 👍

sejas · 2023-05-16T10:13:16Z

It turns out that we already have the setPhpIniEntry step. So I'll go for option 1 and remove the setPhpIniEntry form the new step.

https://github.com/Automattic/wordpress-playground/blob/91a43bf20f6d2780bc4981b32ffc27b208c3ae8a/packages/playground/blueprints/src/lib/steps/client-methods.ts#L32-L37

…to update/wp-now-load-virtually-config-constants

sejas · 2023-05-16T11:09:10Z

@adamziel , I've refactored the code to make defineVirtualWpConfigConsts composable.

adamziel · 2023-05-16T11:32:03Z

@adamziel I think it's the intention as we are defining run-time constants here, not constants that are part of the user's configuration. If users want to define their own constants and have them preserved, they can use wp-config.php which is a default way of doing that.

Oh I see, that makes sense 👍

Still, let's reuse as much code as possible between the two steps, they accomplish mostly the same task after all.

Right now this PR appends if(!defined) define() to a PHP file which means the consts that come later won't have any effect. Let's reuse the same JSON-based approach as in defineWpConfigConsts to enable overwriting them:

	await updateFile(
		playground,
		VFS_JSON_FILE_PATH,
		(contents) =>
			JSON.stringify({
				...JSON.parse(contents || '{}'),
				...consts,
			})
	);

sejas · 2023-05-16T11:34:15Z

packages/wp-now/src/tests/wp-now.spec.ts

 		const requiredFiles = [
 			'wp-content/db.php',
 			'wp-content/mu-plugins/0-allow-wp-org.php',
-			'playground-consts.json',


@wojtekn , I'm removing this .json constants from our tests since they are not needed anymore.

adamziel · 2023-05-16T11:34:31Z

packages/playground/blueprints/src/lib/steps/define-virtual-wp-config-consts.ts

+	let contents = `<?php `;
+	for (const [key, value] of Object.entries(consts)) {
+		contents += `if (!defined('${key}')) {
+      define("${key}", ${JSON.stringify(value)});


There's a phpVars utility for bringing data into PHP code:

import { phpVars } from '@php-wasm/util'; const js = phpVars({ host, port, httpUrl, }); js.host // "127.0.0.1"

Right now it boils down to JSON.stringify, but it will get more powerful escaping at one point.

Thanks @adamziel , I followed the pattern to load a virtual json file.
Now, this step can be called multiple times, and the latest calls will merge previous values and overwrite conflicting values.

I can see that we could refactor defineWpConfigConsts and defineVirtualWpConfigConsts and accept a VFS path in case the user want's to virtualize .json and wp-config.php files. If you like the idea, I can create an issue as a follow-up.

I like that @sejas, thank you for proposing. Let's refactor that! To make sure I follow – they would become the same step, only with a virtualize: true/false option?

Yes, I think the virtualize: true/false option is the best approach.
Another option would be using the given directory and if it's empty, then use documentRoot.
What approach would you like the most?

I'll create the follow-up PR tomorrow.

Let’s use the virtualize option, the directory requires more careful choice from the API consumer when ultimately it’s about a boolean choice

…riables

…to update/wp-now-load-virtually-config-constants

danielbachhuber

I was able to successfully start two sites side by side!

adamziel · 2023-05-16T14:09:25Z

packages/playground/blueprints/src/lib/steps/define-virtual-wp-config-consts.ts

+export const defineVirtualWpConfigConsts: StepHandler<
+	DefineVirtualWpConfigConstsStep
+> = async (playground, { consts }) => {
+	playground.mkdir(VFS_CONFIG_FILE_BASENAME);
+	const jsonPath = `${VFS_CONFIG_FILE_BASENAME}/playground-consts.json`;
+	await updateFile(playground, jsonPath, (contents) =>
+		JSON.stringify({
+			...JSON.parse(contents || '{}'),
+			...consts,
+		})
+	);
+	await updateFile(playground, VFS_CONFIG_FILE_PATH, (contents) => {
+		if (!contents.includes('playground-consts.json')) {
+			return `<?php
+	$consts = json_decode(file_get_contents('${jsonPath}'), true);
+	foreach ($consts as $const => $value) {
+		if (!defined($const)) {
+			define($const, $value);
+		}
+	}
+?>${contents}`;
+		}
+		return contents;
+	});
+	return VFS_CONFIG_FILE_PATH;
+};


@sejas I understand the follow-up you proposed would reduce the duplication, right?

Also, let's make sure any new steps are reflected in pages/blueprints-reference.md

…`defineWpConfigConsts` (#415) Remove `defineVirtualWpConfigConsts` in favor of `defineWpConfigConsts`, adding a `virtualize` option. Asee #343 (comment)

…`defineWpConfigConsts` (#415) Remove `defineVirtualWpConfigConsts` in favor of `defineWpConfigConsts`, adding a `virtualize` option. Asee WordPress/wordpress-playground#343 (comment)

…ineWpConfigConsts step PR #343 added a "virtualize" option so Playground can define new PHP constants without changing the WordPress files in the filesystem. This is incredibly useful. At the same time, mixing defineWpConfigConst calls with and without the "virtualize" option may lead to a situation where some constants are lost or when Playground tries to load a non-existing playground-consts.json file – see #735. Since I can't think of a good rationale to keep the non-virtualized version around, let's reduce complexity and always virtualize the constants. The "virtualize" option will now be noop and is only kept to avoid breaking Blueprint schema validation for existing apps. Closes #735

…ineWpConfigConsts step (#738) ## Description PR #343 added a "virtualize" option so Playground can define new PHP constants without changing the WordPress files in the filesystem. This is incredibly useful. At the same time, mixing defineWpConfigConst calls with and without the "virtualize" option may lead to a situation where some constants are lost or when Playground tries to load a non-existing playground-consts.json file – see #735. Since I can't think of a good rationale to keep the non-virtualized version around, let's reduce complexity and always virtualize the constants. The "virtualize" option will now be noop and is only kept to avoid breaking Blueprint schema validation for existing apps. ## Testing instructions * Confirm unit tests pass * Confirm the [WordPress PR previewer](http://localhost:5400/website-server/wordpress.html) loads Pull Requests without triggering any warnings ## Follow-up work * Adjust Blueprint schema validation to tolerate the `virtualize` option so we can remove it from the type * Call this step `definePHPConsts` as there is nothing specific to `wp-config.php` in it Closes #735

sejas added 7 commits May 15, 2023 19:24

blueprint: create defineVirtualWpConfigConsts to use VFS auto_prepend…

82bff82

…_file to define php constants

wp-now: use defineVirtualWpConfigConsts to avoid writing content to w…

05e0e72

…p-config.php

blueprints: use path.dirname to create the vfs config path

2ee48c5

wp-now: remove empty spaces on import

78f228a

blueprints: add tests for defineVirtualWpConfigConsts

51bcad0

blueprints: defineVirtualWpConfigConsts use VFS_CONFIG_FILE_PATH cons…

80087c4

…tant instead of given path

Merge branch 'trunk' of github.com:Automattic/wordpress-playground in…

ec2cc53

…to update/wp-now-load-virtually-config-constants

sejas self-assigned this May 15, 2023

sejas added the Local Environment i1 label May 15, 2023

sejas changed the title ~~Update/wp now load virtually config constants~~ wp now: load virtually config constants to allow running multiple projects at the same time May 15, 2023

sejas requested review from adamziel, danielbachhuber, katinthehatsite, kozer and wojtekn May 15, 2023 20:01

danielbachhuber reviewed May 15, 2023

View reviewed changes

adamziel reviewed May 15, 2023

View reviewed changes

packages/playground/blueprints/src/lib/steps/define-virtual-wp-config-consts.ts Outdated Show resolved Hide resolved

adamziel reviewed May 15, 2023

View reviewed changes

packages/playground/blueprints/src/lib/steps/define-virtual-wp-config-consts.ts Outdated Show resolved Hide resolved

wojtekn reviewed May 16, 2023

View reviewed changes

wojtekn approved these changes May 16, 2023

View reviewed changes

katinthehatsite approved these changes May 16, 2023

View reviewed changes

sejas added 2 commits May 16, 2023 10:14

blueprints: remove types on docblock

7191df9

blueprints: fix typo

91a43bf

sejas added 2 commits May 16, 2023 12:00

blueprints: make defineVirtualWpConfigConsts composable

cb6ec97

wp-now: update usage of defineVirtualWpConfigConsts

2d8991a

sejas added 3 commits May 16, 2023 12:06

blueprints: update comments

142b514

fix code style

ff0178b

Merge branch 'trunk' of github.com:Automattic/wordpress-playground in…

33a9455

…to update/wp-now-load-virtually-config-constants

sejas requested review from adamziel, danielbachhuber, katinthehatsite and wojtekn May 16, 2023 11:09

wp-now: remove playground-consts.json from tests

6e10e53

sejas commented May 16, 2023

View reviewed changes

adamziel reviewed May 16, 2023

View reviewed changes

sejas and others added 4 commits May 16, 2023 13:18

blueprints: refactor code to load a virtual playground-consts.json va…

ee302e2

…riables

Merge branch 'trunk' of github.com:Automattic/wordpress-playground in…

bebd18e

…to update/wp-now-load-virtually-config-constants

blueprints: remove external path from vite

c7edfa2

Merge branch 'trunk' into update/wp-now-load-virtually-config-constants

821c579

danielbachhuber approved these changes May 16, 2023

View reviewed changes

danielbachhuber merged commit 39dcefe into WordPress:trunk May 16, 2023

danielbachhuber deleted the update/wp-now-load-virtually-config-constants branch May 16, 2023 13:10

adamziel reviewed May 16, 2023

View reviewed changes

sejas mentioned this pull request May 23, 2023

Rrefactor defineWpConfigConsts to work on virtual file system #415

Merged

adamziel mentioned this pull request Nov 13, 2023

[Blueprints] Always virtualize the wp-config.php constants in the defineWpConfigConsts step #738

Merged

wp now: load virtually config constants to allow running multiple projects at the same time #343

wp now: load virtually config constants to allow running multiple projects at the same time #343

Uh oh!

Conversation

sejas commented May 15, 2023

What?

Why?

How?

Testing Instructions

Uh oh!

danielbachhuber left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adamziel commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wojtekn commented May 16, 2023

Uh oh!

wojtekn commented May 16, 2023

Uh oh!

wojtekn left a comment

Choose a reason for hiding this comment

Uh oh!

katinthehatsite left a comment

Choose a reason for hiding this comment

Uh oh!

sejas commented May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Option 1. setPhpIniEntry is an independent step.

Option 2. defineVirtualWpConfigConsts accepts a string

Uh oh!

sejas commented May 16, 2023

Uh oh!

sejas commented May 16, 2023

Uh oh!

adamziel commented May 16, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielbachhuber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

adamziel commented May 15, 2023 •

edited

Loading

sejas commented May 16, 2023 •

edited

Loading

Option 1. `setPhpIniEntry` is an independent step.

Option 2. `defineVirtualWpConfigConsts` accepts a string