VERSION in library declarations does not work for contributed/custom modules

What is the status of this issue now?

I think/hope nod_ has some thoughts about this. IIRC he was the one who pioneered that VERSION constant, and deferred this problem to later.

Don't remember that I introduced it but VERSION should default to the module version in contrib and core version for core stuff. I think we agree on what the behavior is and need to have the code written for that to work.

That's indeed exactly what I expected. Thanks for confirming :)

This seems at least major, because if we don't fix this, some modules may run into problems. OTOH, this shouldn't break anything, nor should it require API changes.

This patch parses the extension's info file to determine version.

If the info file doesn't include a version declaration (e.g. git checkouts directly from source), it will fall back to the version of core. Not sure if there's a better fallback alternative here?

A re-roll, and apparently I had some tests already in progress locally.

Handle non-core, non-active themes properly.

- Fixed handling of core modules (extension is in core/modules, core/themes/, or *.info.yml contains 'version: VERSION')
- Separate tests to check module and theme version handling

Fixed syntax errors

Some refactoring & fixes to tests (now passing locally)

IMHO this is not a bug, but rather how this constant is meant to be used. It is just for the core version.

\Drupal\Core\Extension\InfoParserDynamic::parse has the same kind of logic as the library parser has at the moment. In order for your changes to be useful though we would also somehow take into account git/composer etc.

I continue to see new D8 modules and themes using version: VERSION as a default, without understanding that it resolves to the version of core.

Update patch for 8.5

The fallback logic for the version string added to libraries would be more robust if we were to also include custom values of the deployment_identifier setting in settings.php, and only if deployment_identifier is empty, fall back to \Drupal::VERSION.

In a typical scenario, an automated release procedure bumps the deployment_identifier in settings.php to a specific release number and then all minified/externally preprocessed assets in libraries.yml that don't define their own version would use this instead of \Drupal::VERSION, for improved cache-busting purposes.

Without the deployment_identifier the deployment script must search through all custom modules and either bump the version number in the info.yml and use this patch, or bump versions of specific libraries with externally compressed files that should be excluded from Drupal JS aggregation. Otherwise, the developer is required to bump the version string manually any time a change is made to the files and they're recompressed -- which in reality happens several times a day on each commit/change.

+++ b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php
@@ -99,9 +121,26 @@ public function buildByExtension($extension) {
+            if ($extension_type === 'theme') {
+              $extension_info_path = $this->themeHandler->getTheme($extension)->getPathname();
+            }
+            else {
+              $extension_info_path = $this->moduleHandler->getModule($extension)->getPathname();
+            }
+
+            $info = $this->infoParser->parse($extension_info_path);
+            if (!empty($info['version'])) {

this shouldn't be parsing the extension, it should instead use the various extension list services.

Other than that, this looks like a nice improvement.

I'm not sure this should be classed as a bug though, more of a feature request or task.

Triaged it as part of Bug Smash Initiative.

Going to move it to a task. Feel free to counter with alternate reasoning as to why its a bug.

@dawehner above also agreed with me re the Bug classification

Parsing it ourself here bypasses hook_system_info_alter and is also less performant.

Haven't read yet over all comments but found parallel thoughts of what brought me here in my mind matching the initial issue summary point 1.) and comment #3 or nod_ while I was fixing library.yml version issues in contrib modules.

What if we simply use the module version created by packaging script as /?version-path-suffix for library files and deprecate the tag for library files completely? Is that possible? It would be a quick solution at least ... (Drawback: no manual override possible then. But for what should that be needed? And!-> git installations would be out of scope... maybe a deal breaker)

Hi,
I've rerolled the patch in #21 with #31 suggestions, with Drupal 10.4.x

Have not reviewed but change should be in an MR.

Added the Mr with the Drupal 11 version with the some code updates and Added Mr Form Patch #38 as per the suggestion of 40 .

added template to help with the issue summary update.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.