Skip to content
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

Load zc_plugin directories in a predictable order #6540

Merged
merged 4 commits into from
Jul 5, 2024

Conversation

lat9
Copy link
Contributor

@lat9 lat9 commented Jun 15, 2024

Currently, zc_plugin files are loaded in the order in which they were installed, leading to an unpredictable load order.

This PR loads them, instead, by sorting the installed plugins' unique_key.

lat9 added 2 commits June 15, 2024 08:44
Currently, `zc_plugin` files are loaded in the order in which they were installed, leading to an unpredictable load order.

This PR loads them, instead, by sorting the installed plugins' `unique_key`.
@drbyte
Copy link
Member

drbyte commented Jun 18, 2024

I gather that the intent of this load-order change is specific to "booting up" the (installed) plugins on page-load.

The Admin UI for Plugin Manager would require a different change for actually sorting the list of displayed "available, whether 'installed' or not" plugins:
@zcwilt ... would changing PluginManagerDataSource.php be safe (no undesired side-effects in other lookups?), done this way?:

    protected function buildInitialQuery() : Builder
    {
-        return (new PluginControl())->query();
+        return (new PluginControl())->query()->orderBy('name')->orderBy('unique_key');
    }

@zcwilt
Copy link
Member

zcwilt commented Jun 18, 2024

@drbyte I have no issue with that change, although see no benefit in it, had always wanted to get back to the ViewBuiolderrs stuff to tidy up and add sort filters etc.

@zcwilt
Copy link
Member

zcwilt commented Jun 18, 2024

as for the overall change, I would ask, what problem is this solving. What use cases mean we need to load plugins in a this specific order.

Just to be clear. I agree that there maybe specific reason you want a plugin to load before another plugin,
I can imagine a plugin that provides a set of utility classes that another plugin might want to use and not have to re-implement them.

I did consider writing code to do some kind of dependency check based on a manifest entry, but at that point you are pretty much writing a custom Composer clone when why not just use Composer.
(Well apart from then having to have Service Provider classes, DI containers blah blah)

@lat9
Copy link
Contributor Author

lat9 commented Jun 18, 2024

Mostly thinking about language-file load order. Granted, plugins shouldn't be supplying the same filenames, but just in case there's a rogue this change ensures that the result is the same from site to site, regardless the order in which the plugins were installed.

@lat9
Copy link
Contributor Author

lat9 commented Jun 18, 2024

I gather that the intent of this load-order change is specific to "booting up" the (installed) plugins on page-load.

The Admin UI for Plugin Manager would require a different change for actually sorting the list of displayed "available, whether 'installed' or not" plugins: @zcwilt ... would changing PluginManagerDataSource.php be safe (no undesired side-effects in other lookups?), done this way?:

    protected function buildInitialQuery() : Builder
    {
-        return (new PluginControl())->query();
+        return (new PluginControl())->query()->orderBy('name')->orderBy('unique_key');
    }

That was also what I was looking for (too many buried classes). Without that change, the Plugin Manager displays the plugins in the order in which they were installed:
image

With that change, they're now displayed alphabetically:
image

@drbyte
Copy link
Member

drbyte commented Jun 18, 2024

That was also what I was looking for (too many buried classes). Without that change, the Plugin Manager displays the "plugins in the order in which they were installed":

Clarification, because "installed" is a multifaceted word; so I think what you meant was: "... displays the plugins in the order they were added to the filesystem"

@scottcwilson
Copy link
Sponsor Contributor

Is this ready to be merged?

@lat9
Copy link
Contributor Author

lat9 commented Jul 5, 2024

Is this ready to be merged?

In my opinion, Yes.

@scottcwilson scottcwilson merged commit 2fa3d15 into zencart:master Jul 5, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants