WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#32468 closed feature request (duplicate)

Add Registry Class as WP_App

Reported by: jacobsantos Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: General Keywords: close
Focuses: Cc:
PR Number:

Description

Registry is a design pattern that takes the place of globals. It is a singleton with possible helper methods for common names for retrieval. You could just call it WP_App or \WordPress\App, provided WP_App or similar does not already exist, which it currently does not as of r32519.

There are possible extensions to add to a Registry object. My idea would be to provide a test callback to ensure that values set on the registry are correct and reject values or objects that are incorrect.

The main purpose of the class is to be a container for other objects that can be retrieved through a common interface. Also, it would allow for replacing the implementation provided the contract is satisfied.

A notable example would be Laravel that uses a facade and registry pattern through \Illuminate\Foundation\Application and \Illuminate\Support\Facades\* classes. This provides a common quick and easy way of initializing library classes and replacing library classes with other user implementations.

Benefits

  • Removes dependency on globals, which do I need to further explain the benefits of this? I am capable of doing so, if you ask.
  • Removes dependency on a specific class, instead of initializing a class, you can simply call into the registry and pull the existing instance or have the instance created for you.
  • Registry implementation is on a whole a common design pattern and is often small and without external dependencies. There are many examples of how the Registry pattern works, which should reduce the training needed for maintaining and implementing such a pattern.
  • Could be extended to provide backwards compatibly with plugins and external code that still relies on globals.
  • New code wouldn't have to use globals, there would be a standard way of registering and retrieving values.
  • This would provide a new way of replacing existing implementation. So it would reduce the need to modify core code.
  • Building off the of the previous benefit, code can be provided as a proof of concept that either fixes a problem or extends WordPress with the proof of concept that can be loaded in a plugin and easily and quickly determined to be of value.
  • Building off of the "replacing existing implementation", when testing, mocks can be provided as implementation instead of having to rely on other means of mocking or removing dependence allowing test code to focus on fewer areas when integration or system testing.

History

I realize this has been proposed before and turned down for various reasons. I think those reasons can and should be debated. I forget what the tickets were and what all of the previous objections were. My guess is that if WordPress must still run on PHP4, then this proposal will again be rejected.

Aside

This includes the additions of interfaces and therefore existing code would be refactored to include interfaces. I'm not suggesting that current code be moved over to this as part of this proposal, I'm only suggesting that later patches in other tickets, if this ticket is approved and the patch included in a future release would be part of the improvements of WordPress.

Another extension would be the blocking of classes that replace critical pieces of WordPress. This should be possible, but I propose that this be in another ticket along with the implementation. The discussion, I hope to be kept to the presence of a Registry in WordPress and the ways it could be used in core and in plugins and themes.

Possible Objections

  1. Adds New Code

    WordPress has over a million lines of code. Adding 100 to 200 with the above benefits seems like a good trade off to me. My implementation looks nothing like the 1000+ line Laravel Application class. Granted, the class will, of course, have new code added to it over the course of time.
  1. Global effect on testing

    My proposal says nothing of modifying existing code and only suggests that existing core code could be modified in the future to use the proposed code. The implementation will have unit tests (actual unit tests) provided and integration tests can further be provided once core code is moved over to use the proposed Registry class.

    In all, the proposal should, if done correctly, have no effect on core code or testing, it should work as if nothing was changed. Furthermore, test cases should be provided that demonstrate how modifying the existing implementation could work.
  1. Security

    As globals can be replaced now anyway, there is a potential to prevent critical pieces of code from being replaced or changed.
  1. PHP4

    WordPress, I believe, no longer supports PHP4 and therefore backwards compatibly shouldn't still be a concern. If it is, then why? Is there enough of an audience that PHP4 is still a concern? If so, then I believe they have other concerns than whether WordPress can still run. For example, "Why is my server hacked and what I may I do about that?" To which my suggestion would be to upgrade PHP to a more stable and more known secure version as a start. Or preferred to a later version of the distro, and upgrade all possible attack vectors to a more known secure version.
  1. Mere OOP Design Aesthetics

    Suggesting that this be denied merely on the basis that it is, in your opinion, design aesthetics with little to no benefit, belittles the future benefit it provides and the first step of improvement needed to further WordPress development.

    Simply because the addition may not make sense at the current time doesn't mean it won't be accepted in the future. The question in the future might be, "Why didn't this exist prior?"

    Too often the complaint is that it doesn't make sense to you right now, with the result that the person proposing the solution and offering the patch(es) moves on with their life. While eventually, it does start to make sense to you and the person offering the solution is long gone. Someone might come along, but if WordPress doesn't have this or something similar already, then I doubt that was the case.

    If the benefits above are not enough to convince you of why this should be included, then there is nothing I can say that may convince you otherwise. My only suggestion is to find the answer on your own. I think that sometimes, a person needs to discover the answer on their own, because no amount of words or example will convince that person otherwise. If it helps, I thought writing test cases were a terrible idea, until I actually wrote some, then I wondered how I could have lived without them.

    Or I guess another alternative is to go to someone you trust and have them either explain it or give a better solution. Given that you are skeptical of my claims.

I need this proposal, because it is the foundation of which future patches and proposals will be built. The other proposals are possible, but they will not flow as cleanly as if they had this proposal to build upon. I understand this proposal needs to stand on its own and it might be a version or two before it is actually used depending on the time it takes for refactoring core or plugin developers to start using it.

Change History (7)

#1 @jacobsantos
4 years ago

  • Type changed from defect (bug) to feature request

#2 @chriscct7
4 years ago

It would need to be WP_App as \WordPress\App isn't usable since WordPress supports 5.2 as a minimum version for PHP and namespacing wasn't added to the PHP spec until 5.3.0

#3 @chriscct7
4 years ago

  • Keywords dev-feedback early added
  • Version trunk deleted

#4 follow-up: @ericlewis
4 years ago

Hi @jacobsantos, thanks for your proposal! The registry pattern is chill, although, as you've hinted at, jamming it into WordPress without obvious, practical gains is going to be a non-starter.

This would probably be better to consider when we're building out new functionality, rather than as a blanket proposal.

You mention

As globals can be replaced now anyway

how can globals be replaced without breaking everything?

#5 in reply to: ↑ 4 @jacobsantos
4 years ago

You need a registry to exist, so that classes may start to use it. You need classes that use a registry in order for the registry to exist. That is an interesting problem.

Note: I use "unit tests" below. What I mean is the testing and running a single unit at a time. Not the current WordPress test suite known as "unit tests" which weren't unit tests.

Replying to ericlewis:

Hi @jacobsantos, thanks for your proposal! The registry pattern is chill, although, as you've hinted at, jamming it into WordPress without obvious, practical gains is going to be a non-starter.

No, I'm pretty sure I listed practical gains that were not obvious. If they were obvious, the pattern or a similar or more advanced pattern would already be in WordPress. The problem is that writing unit tests produces code that is unit testable. Unless you care and write unit tests often, then the gains won't be obvious. Other proposals were going to be submitted that depended on this.

This would probably be better to consider when we're building out new functionality, rather than as a blanket proposal.

If the environment is using globals for everything, then when will you move from globals? The proposal was part of a push to move WordPress to use better code that is more testable.

You mention

As globals can be replaced now anyway

how can globals be replaced without breaking everything?

Exactly. A plugin could replace a global and either break everything or change the behavior. It is also possible to replace a global that is an object with a partial implementation that works for the developer, but crashes on a client.

Using a registry along with an interface would provide for a way to ensure that all available methods exist for an object. Without having to result to the wasteful if method exists conditions.

Last edited 4 years ago by jacobsantos (previous) (diff)

#6 @ericlewis
3 years ago

  • Keywords close added; dev-feedback early removed

I'm not sure there's much actionable here.

#7 @swissspidy
3 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

#37699 has the same goal and lots of traction already.

Note: See TracTickets for help on using tickets.