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

Allow optional dependency / one-of-several dependency requirements to control extension boot order #1318

Closed
tobyzerner opened this issue Dec 25, 2017 · 8 comments · Fixed by #2629

Comments

@tobyzerner
Copy link
Contributor

Currently extensions are booted in the order that they are enabled (in both backend and frontend).

This should be modulated so an extension cannot be booted before any of the extensions it requires.

@tobyzerner tobyzerner added this to the 0.1 milestone Dec 25, 2017
@luceos
Copy link
Member

luceos commented Dec 26, 2017

Isn't this what $defer is meant to do with a ServiceProvider? I know we can make this easier, but classifying it as a bug seems a bit harsh taking the previous into account.

@askvortsov1
Copy link
Sponsor Member

IMO this is fine as long as extensions are enabled in the proper order (which is a larger issue I feel). I'd propose handling this by adding in a check when enabling an extension that all its dependencies are enabled, and when disabling an extension that there isn't anything dependent on it enabled. Thoughts?

@luceos
Copy link
Member

luceos commented Mar 6, 2020

Can LifecycleHooks have a mechanism that allows extensions to be loaded before or after other extensions?

Eg:

class LoadingSequence implements LifecycleHook
{
    public function before(): ?array {
        return ['flarum-tags', 'fof-upload'];
    }
    public function after(): ?array {
        return ['flarum-flags', 'fof-byobu'];
    }
}

@askvortsov1
Copy link
Sponsor Member

I was thinking of doing it directly through composer reqs, but doing it explicitly like this might be a good idea too. Both have merits, not sure which is preferable

@askvortsov1 askvortsov1 changed the title Make sure extensions are only booted after their dependencies Allow optional dependency / one-of-several dependency requirements to control extension boot order Jan 12, 2021
@askvortsov1
Copy link
Sponsor Member

A few more thoughts:

  • Frontend intializers should be booted in the same order as backend code to ensure consistency, discussed more in Javascript initializer doesn't have access to certain data issue-archive#169.
  • Do we need to allow specifications of extensions the current extension should be booted before? I don't think that a package should be responsible for knowing which other packages depend on it: It makes the dependency graph a lot more complicated.

To that end, I would propose that we add a "optionalDependencies" key to the flarum-specific config, which would be an array of extension IDs. We would then modify the calculateDependencies method of Extension (https://github.com/flarum/core/blob/a56ccee2da40b6355d83ae2fb18807ebae6054ad/src/Extension/Extension.php#L200-L216) to also include any of those optional dependencies that are installed. Then, we would implement #2317, and run it whenever an extension is enabled or disabled, and as a composer script for flarum/flarum. All-in-all, this would give us a fairly robust dependency system.

However, this only solves optional dependencies, not "one-of-some" dependencies (a "one-of-some" dependency could be https://github.com/Xelson/flarum-ext-chat depending on either pusher or websockets). SInce there are several options, they can't be included in require, which makes this a bit more complicated. However, this could also be implemented as part of (or as a followup to) the above optional dependencies system.

@SychO9
Copy link
Member

SychO9 commented Jan 19, 2021

I would be in favour of using an optionalDependencies key, it's the simplest and most optimal solution.

However, this only solves optional dependencies, not "one-of-some" dependencies (a "one-of-some" dependency could be https://github.com/Xelson/flarum-ext-chat depending on either pusher or websockets). SInce there are several options, they can't be included in require, which makes this a bit more complicated. However, this could also be implemented as part of (or as a followup to) the above optional dependencies system.

While I haven't used it before myself, this might be solvable by using virtual packages in composer: https://getcomposer.org/doc/04-schema.md#provide

Example of a package using this: https://packagist.org/packages/symfony/cache

So I think pusher and websockets would both provide for example flarum/websockets-implementation and the dependent extension would require that package name, and I think composer takes care of the rest.

@askvortsov1
Copy link
Sponsor Member

So I think pusher and websockets would both provide for example flarum/websockets-implementation and the dependent extension would require that package name, and I think composer takes care of the rest.

Ah, and then both could be specified as optional dependencies! As a result:

  • At least one must be installed
  • The dependent package will only boot after whichever one is installed has booted.

Brilliant!

This was referenced Jan 26, 2021
@askvortsov1
Copy link
Sponsor Member

The remaining item here is to add a composer script to flarum/flarum to rerun dependency resolution when a package is removed. Then we'll have a fully robust system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants