WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.48k stars 4.18k forks source link

Why are there PHP files in the JS packages on npm? #33241

Open azaozz opened 3 years ago

azaozz commented 3 years ago

Been wondering about this for a while. Seems these PHP files were added as a "quick fix" during the first merge of Gutenberg to WP 5.0, but not sue anybody has ever looked at "how to" and "why" since.

As far as I see it is not a good idea to (auto)add PHP code from the Gutenberg plugin to core as some of that code serves a bit different purpose in the plugin. There are certain, sometimes significant differences when a function or a class is included in core compared to when it is added by a plugin. For example a plugin may want to be backwards compatible with previous versions of WordPress, etc. Such code would be "dead" in core.

I understand some of that PHP code is generic and may be needed by third parties. Perhaps those files are okay to include in the packages, as long as they are not WP specific. On the other hand WP specific PHP code, especially code that is needed for the plugin to work in older WP versions, should not be part of the npm packages. It is useless for third parties and not suitable for core. Seems such "compatibility" PHP code should be in /lib and loaded conditionally on the WP version or function_exists(), etc.

gziolo commented 3 years ago

It's the simplest way to keep the PHP files for core blocks in sync with WordPress core: https://github.com/WordPress/wordpress-develop/blob/73e24c997c1182ac48856c1546cf4719b51ae27d/tools/webpack/packages.js#L174-L181

The thing is that all core blocks that contain PHP code can be used only in the context of WordPress. We can always extract all general blocks that work in the standalone block editor like Paragraph, Heading, List, etc.

The only exception is @wordpress/block-serialization-default-parser/parser.php that could be as well moved to the WordPress core and maintained there.

azaozz commented 3 years ago

It's the simplest way to keep the PHP files for core blocks in sync with WordPress core

Yeah, on first look it seems that way. Looking a bit more, what happens when the (PHP) code needed for the Gutenberg plugin is different from the code needed for core? Who "wins", the plugin or core? :)

There have been few related bug reports recently: https://core.trac.wordpress.org/ticket/53610 https://core.trac.wordpress.org/ticket/53410 https://core.trac.wordpress.org/ticket/53432

Seems that for now the plugin "wins". That benefits some of the ~300,000 plugin users, but may cause errors or is "dead code" for the (potential) ~60,000,000 WP sites. Don't think it is a good solution, and it looks like there will be more and more similar cases in near future.

There seem to be two options to improve that:

  1. Stop including the PHP files in npm packages and merge them "by hand" if/when there are any updates. That will allow removal of the dead code and other adjustments that may be needed when moving PHP code from a plugin to core.
  2. Ensure that all the PHP that is included in npm packages is strictly intended for the current core trunk. That may make some of these files not usable in the plugin. In these cases any compatibility code should go in /lib.

There are some other concerns too. Newer version of the plugin running in older version of WP would probably work properly. However older version of the plugin in a newer version of WP may run into difficulties and/or conflicts with other plugins.

One way to try to prevent such problems before they happen would be to never use in Gutenberg the PHP files that are intended for core and are included in the npm packages. Instead, when the plugin is released, all the PHP needed for it should be "forward-compatible" with fewer versions of the code that may already be loaded and preferably be in /lib.

youknowriad commented 3 years ago

Thanks for raising this, I think it's an important subject that deserves some improvements. Personally, I think we shouldn't stop sharing files, I think we should actually share more files. If you've been involved with backports to WordPress you know how much painful non-js painful are already and stopping the blocks from sharing their code will just make it worse.

That said, I think it's be good to find better alternatives to what we already do, and for me there's another alternative worth that wasn't mentioned ... packages (like in php packages) :)

Backporting JS packages automatically has been working great for some time now, why not do the same for php ones. Organising the php code in packages forces the developer to think about third-party (aka core) usage and not just the usage in the context of Gutenberg, meaning the code could become more reusable and easier to integrate. (Not everything need to be a package though in php obviously)

For me there are things that could clearly be easily shared without too much effort:

Historically, WP didn't load any external php package to its code base and it might be an issue but why not start with WP's own packages.

oandregal commented 3 years ago

I'd like to share a use case that would benefit greatly from an improved mechanism to reuse PHP code across codebases: we have a piece of code to extract strings for translation from the theme.json file. It's the same in 3 different codebases: WordPress core, the Gutenberg plugin, and the wp-cli. Doing this copy over manually has proved very time-consuming to maintain and update.

azaozz commented 3 years ago

backports to WordPress you know how much painful...

Yeah, it is in some cases. The pain mostly comes from the differences in the PHP code between core and the plugin.

packages (like in php packages)

Hmm, in theory packages should work, but there will be only one "consumer": WP core. As far as I see they won't be reusable in the plugin (in most cases), and some (many?) will be compatible only with a specific WP version. Wouldn't it be cleaner and easier to just commit them to core trunk?

there are things that could clearly be easily shared without too much effort

True, if the PHP code is quite generic, it may be possible to reuse it in different WP versions. However not in the plugin where "forward compat" will probably still be a problem in some cases.

improved mechanism to reuse PHP code across codebases... ...WordPress core, the Gutenberg plugin, and the wp-cli

Sharing that should work as long as the piece of code is generic and doesn't depend on other WP functions. At the same time if it exists in core, both the plugin and cli will likely be able to use it.

Doing this copy over manually has proved very time-consuming

Hmmm, copying and pasting of some code, testing it, and releasing it may be time consuming, but not sure there will be any significant difference if it was a package. The only thing would be that the code from the package is "copied" and "pasted" automatically. That would probably take no more than few seconds to do "by hand" :) Testing the changes locally and releasing the update is the bulk of the work.

WordPress had something like a package many years ago. The WP_Dependencies classes and functions were reused in couple more places as far as I remember. Eventually that was abolished as the changes needed for core were not needed elsewhere, and started to introduce bugs.

azaozz commented 3 years ago

There are some other, more "drastic" options: