Do not use VERSION string in libraries definition

Prem Suthar → made their first commit to this issue’s fork.

I have Remove the VERSION string in libraries definition and Raised the Mr Please review.

Looks good to me, thanks.

Note that this will leave a mix of some library definitions without a version number, and some with, because leaflet seems to be declaring lots of versions like: 1.x etc.

Anyway, this is an improvement, maybe we need to see if a module maintainer would like to remove all the version numbers, or start using them an incrementing them when something in a library file changes.

Thanks a lot Steven ... that all makes sense,
and also according to this article from Chromatic: https://chromatichq.com/insights/drupal-libraries-version/

With the new attached patch I comply with your suggestion and fix all the not Remote libraries versioning, aligning them with the current dev branch.

Please QA and Review this attached Patch (which is more accurate than the provided MR !43), if you have some more time on this ...

@itamair thanks. I think you've missed the leaflet-drupal library from your patch though?

https://git.drupalcode.org/project/leaflet/-/blob/51a59100f864a16fb28e0b...

If you are wanting to go with option 1 and include the simple version number, are you going to increment it in the definitions when you make changes to the JS/CSS files?

Thanks Steven ... yer right you are, I missed the leaflet-drupal library version fix/update,
thus here it is a new attached patch, that includes also it.

For what regards the naming approach I am taking inspiration from the Geolocation module that is using as version the same dev branch name:
https://git.drupalcode.org/project/geolocation/-/blob/8.x-3.x/geolocatio...

This might have the advantage not to need an update of the version for each new release of the module, and of the specific js/css release and I am assuming it will behave similarly to the option 2 that you mentioned and the js/css caching system will look for hashed changes in each library?
Or would I be too much optimistic on this?
What would be the behaviour in case of this naming simply based on the module release dev branch?

@itamair sadly, there's no magic in Drupal around the handling of these strings. So, no it's not going to do anything special with 10.1.x or whatever.

Either:

• You should simply remove the version strings from the library definitions, and then when core needs to, it'll get PHP to hash the file to generate a version string. This will have a very small performance penalty.
• You should change the version string every time you change the contents of the files. Easiest way it to set it to be a number, and increment it when you change the contents. There's no need for it to be semver etc. You do not need to change the version on every release, if the file contents hasn't changed.

thanks @steven ... for your detailed and clear explanation.
I would go for the simplest solution, also from the maintenance effort side (that is already quite demanding an all this geofield stack modules), hence simply remove all the versioning tagging and management for libraries that are not remote/external ones,
considering that you don't highlights any caching side effect doing this, but rather a proper and automatic handling from the PHP cashing/cashing system ... isn't it.

Please, give me a final green flag on this new attached patch, or let me know what might be stil missing.
(and super thanks again for your guidance ...)

Sorry, new final patch, that re-adds the leaflet-geoman library version tag (removed by mistake) ...

@itamair yeah, this looks great to me now. Thanks!

Great! New leaflet 10.2.24 → just released with all this ...

Automatically closed - issue fixed for 2 weeks with no activity.