[Blueprints] Always virtualize the wp-config.php constants in the defineWpConfigConsts step #738
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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
adamziel
added
[Type] Bug
Collaborator
Author
|
cc @sejas @danielbachhuber @wojtekn for visibility as this is relevant for wp-now. No changes required on your end. |
Collaborator
Author
|
I'll go ahead and merge. |
adamziel
added a commit
that referenced
this pull request
Nov 17, 2023
#738 introduced a change that made all WordPress constants be defined by including a "virtualized" wp-consts.php file. However, the required `setPhpIniEntry()` call was sometimes happening after the initial `php.run()` which made it noop. This broke the networking support merged in #724. This commit solves that by exposing a new `php.defineConstant()` API that handles the file virtualization. I'm not super happy about virtualizing files so let's revisit that in the future, see #750.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Follow-up work
virtualizeoption so we can remove it from the typedefinePHPConstsas there is nothing specific towp-config.phpin itCloses #735