Problem/Motivation
This module depends on the external jQuery Validation library. This library cannot be committed to the Clientside Validation module repository because:
- Drupal.org has a strict GPLv2+ policy for the drupal.org contributions repository, which conflicts with certain libraries.
- Modules and themes on drupal.org and their release cycles should not be affected by the release cycles of external libraries.
Currently, the Clientside Validation module's README recommends installing the jQuery Validation library by "extract[ing] the zip file to the js/lib of this sub module". However, is not recommended because:
- Best practices for module upgrades dictate to delete the old module folder before the upgrade. This removes any libraries inside of that folder.
- Multiple modules can implement the same library.
- In this particular module, a
.gitignorefile makes it difficult to commit the jQuery Validation library to the project, which makes it difficult for site builders who use Git to manage files on a website, such as on Acquia Cloud and Pantheon, and some self-hosted setups.
Drupal core / the official Drupal 8 documentation does not specify where modules should be placed. However, it seems to have become convention (see the modules: leaflet, slick, blazy) to locate libraries relative to the Drupal root at /libraries/$library_machine_name. Note, however, this requires a change to the path specified for the library in the module's *.libraries.yml file: paths to library files must begin with /libraries/$library_machine_name/.
For more information, please refer to the *.libraries.yml files of the aforementioned contributed modules, and the documentation for LibraryDiscoveryParser::parseLibraryInfo().
Proposed resolution
Update the jquery.validate definition in clientside_validation_jquery/clientside_validation_jquery.libraries.yml to look like:
jquery.validate:
js:
/libraries/jquery-validation/dist/jquery.validate.js: {}
Modify the README to specify where to put the library. See the Leaflet module's README for inspiration.
Update the .gitignore file at the root of the project, and delete the clientside_validation_jquery/js/lib/.gitkeep file.
Remaining tasks
Write a patch- Feedback and review
- Feedback and RTBC
- Commit and release
- Update project page to specify where to put the library
User interface changes
None.
API changes
None.
However, it does change where the module expects the library to be, so it will break people's existing installations. This will almost certainly warrant a note in the module release notes, but there doesn't seem to be a convention on whether this would also warrant a new major or minor version number, a new hook_update_N() that displays a message, a message in the status report if the library is detected at its old location, a message displayed to administrators on every page load until the problem is fixed, or some combination of these. Feedback on the module maintainers' preference would be appreciated so that could be added to the patch!
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | 2895666-20.patch | 8.2 KB | nikunjkotecha |
| #17 | interdiff.txt | 4.91 KB | dsdeiz |
| #17 | clientside_validation-library-2895666-17.patch | 7.32 KB | dsdeiz |
| #15 | 2895666-15-allow-library-to-be-installed-outside-module.patch | 3.79 KB | nikunjkotecha |
| #2 | 2895666-2-allow-library-to-be-installed-outside-module.patch | 3.49 KB | mparker17 |
Comments
Comment #2
mparker17Patch attached. Feedback welcome!
Comment #3
mparker17Oops, forgot to set status and assigned fields after uploading patch.
Comment #4
nikunjkotechaAdding condition for library to ensure both CDN and library both are not added.
Comment #6
nikunjkotechaFixing build error.
Comment #7
nikunjkotechaComment #9
nikunjkotechaNot sure how to fix this error, will leave it to patch added in #4
Comment #10
nikunjkotechaUse the same name as in current module's composer.json to allow hack in https://www.drupal.org/node/2784069#comment-11922633 work
Comment #11
nikunjkotechaUpdated even version to match what is there in module.
Comment #12
nikunjkotechaComment #13
nikunjkotechaComment #14
nikunjkotecha@attiks / @Jelle_S
Please confirm the problem and solution for this one.
Comment #15
nikunjkotechaFix issue in libraries.yml
Comment #16
jelle_sThis might fail on existing installations:
if
file_exists('libraries/jqueryvalidate/dist/jquery.validate.js')returns FALSE (which it will for existing installations) andfile_exists(drupal_get_path('module', 'clientside_validation_jquery') . '/js/lib/dist/jquery.validate.js')returns TRUE (which it will for existing installations that do not use the CDN), no js paths are altered and we'll try to load'libraries/jqueryvalidate/dist/jquery.validate.js, which doesn't exist.We might also want to add a hook_requirements so we can trigger a warning (maybe even error?) so that we can deprecate putting he library inside the module folder in the future?
Comment #17
dsdeiz commentedHere's my take on the requests. Not sure if I got it right though.
Comment #18
nikunjkotechahi @dsdeiz, thanks for the updated patch. There is still a typo and now that we are fixing this, I would love to use the latest version in instructions. Will update patch and push soon.
Comment #19
nikunjkotecha-
Comment #20
nikunjkotechaComment #22
nikunjkotechaCreated tickets for further work: