NEW: Add Config::moduleExists()

[kinglozzer]

Directory-based check for whether a module exists in the manifest. Uses the same method of checking that the YAML moduleexists: foo does, just caches it and adds a way of accessing it through Config.

Usage:

Config::inst()->moduleExists('sqlite3'); // boolean

Thinking: class_exists() is obviously the best way of doing this, but classes can be (and are) added/removed from some modules, so this can sometimes be a more robust check. Plus, the functionality to check already exists - it just can’t be accessed currently.

[sminnee]

One thing I wonder for future proofing: should we recommend that people use the full Composer name of the module? If, in the future, we shift to putting modules into the vendor/ directory, then we will have created further BC issues by introducing this patch.

For now, the code would have to simply ignore the vendor prefix, and we might make the vendor prefix recommended rather than required.

What do others think? Should we future-proof this API change?

[kinglozzer]

I think it’d be a good idea to do what you’re suggesting. To clarify, we’d store (and check against) in the following order:

1. Full composer name: vendor/silverstripe-package
2. Package name without vendor: silverstripe-package
3. Directory name (in case set with installer-name, or not installed with composer): package

Is that along the lines of what you were thinking?

[sminnee]

That seems reasonable; having 3 different things to check is a bit of a code-smell, but I'm not sure which of those options we can drop.

Maybe #2 should be dropped?

[kinglozzer]

I think we can safely drop 2, yeah. 1 adds the new functionality and 3 provides full backwards compatibility.

[willmorgan]

Thrice!

[willmorgan]

Just a thought... what about namespaces? As long as we're definitely behind Composer I see no problem with the (amended) approach above though.

[kinglozzer]

@willmorgan Do you mean if two namespaced modules share the same directory? We can’t (currently) support that anyway (conflicting _config.php/yml etc).

[kinglozzer]

Just re-thinking this, would we want to:

1. Parse each docroot/module/composer.json for the composer name, only modules that are detected as SilverStripe modules (i.e. directories containing _config.php/_config/) are included.
2. Parse docroot/composer.lock, which provides more info (like which version of a module is installed) and also includes non-SilverStripe modules.

I’m leaning towards the former: it’s probably simpler, it matches the current behaviour, and the latter option would include composer/installers as a module.

---

One potential issue: let’s say I write a module that can be installed either with or without composer, and I want to check if gridfieldextensions is installed. I’d ideally want to check for both ajshort/silverstripe-gridfieldextensions and gridfieldextensions to cover both eventualities, but there’s currently no way to do this. My initial idea was this:

moduleexists: 'ajshort/silverstripe-gridfieldextensions', 'gridfieldextensions'

However I think this syntax implies that it’ll check if both of these exist. Do you think we could check for docroot/composer.lock and store the appropriate module identifier based on the presence of that?

[dhensby]

Just looking at this, what's the real purpose of this feature? Is it so you can provide functionality with several interchangeable modules?

Doesn't this make dependency resolution for your package tricky... You'd have to use suggested dependencies, you'd also have to set a priority for which you prefer if both are available.

Other than that, I don't see too much value in having this feature...

Scanning composer files or the lock seems pretty nasty and relies on composer being used. If composer has been used, cant we just use a composer class to pull out all the modules? Further, what about modules that aren't pulled in with composer (eg: custom internal modules)?

[kinglozzer]

@dhensby If I remember correctly it was that I wanted a code-based check for whether a module existed (can’t remember the exact use case).

Further, what about modules that aren't pulled in with composer (eg: custom internal modules)?

The intention was to fall back to directory names (which I believe is how the YAML system currently works).

That said, I don’t think it’s a crucial feature at all, so I’m happy to leave this - it just stood out to me at the tune that you can set values in YAML based on whether a module exists yet can’t access that information in PHP.

[dhensby]

Further, what about modules that aren't pulled in with composer (eg: custom internal modules)?

The intention was to fall back to directory names (which I believe is how the YAML system currently works).

I think that makes sense.

All in all, I think that this feature is going to take up too much time and effort to justify it - I don't see there's a big need for it. You should be able to use your own Config class that could have this feature, if it were essential to a project, other than that I'd recommend devs use composer to define dependencies and use class_exists() on dependant classes (if they aren't forcing dependencies with composer). Ultimately, you'll rarely depend on a module by name, you'd depend on the classes it provides.