Support local library file

Add link to article about CDN.

Include adding the file directly in the module, as a proposed solution.

Since it is just a single file (js.cookie.min.js), the Tagify solution method of checking for a local library file might be overkill, so here's a proof of concept, with the file included in the module.

Update issue summary, adding inclusion of the file in the module as an option.

Tested this on Drupal 10, I can see that the new library has been added in the module.

Thanks for confirming @danrod. If you want to check if the JS Cookie module is loaded and works in another module, you can try https://www.drupal.org/project/autologout/ → which requires the JS Cookie module.

@danrod: I just found out including the file is discouraged, so we probably need to add support for local file, checking if it is available and use the CDN as fallback.

I just discovered that the module already supports a local file, with the CDN as fallback, so the task now is documenting it.

Here's a one-liner, which will download the file and create the needed directories, if they don't exist:

curl --create-dirs -o libraries/js-cookie/dist/js.cookie.min.js https://cdn.jsdelivr.net/npm/js-cookie@3.0.5/dist/js.cookie.min.js

PS. Use the dev-release of Automated Logout → to check if JS Cookie module is loaded, the current release (8.x-1.4, July 2022) doesn't use it.

I'm coming from autologout #3449275-15: JS Cookie needs to be enabled automatically when updating →
To avoid CDNs (not compliant with our CORS policy) I added js-cookie as a composer repository.

{
    "type": "package",
    "package": {
        "name": "js-cookie/js-cookie",
        "version": "3.0.5",
        "type": "drupal-library",
        "dist": {
            "url": "https://registry.npmjs.org/js-cookie/-/js-cookie-3.0.5.tgz",
            "type": "tar"
        }
    }
}

Then composer require js-cookie/js-cookie

PS: the github release assets of js-cookie do not contain the compiled zip, not sure why... so I used npmjs here.
----

Perhaps we could add a composer.libraries.json and document in the readme to either use wikimedia/composer-merge-plugin or add the repository in your main composer.json.
Several projects do this, e.g: https://www.drupal.org/project/anchor_link →

Thanks @herved, that's another option. Alternatively, contrib modules could pragmatically bundle scripts directly in the module, making it easier for the end users? Of course, the module maintainer then need to make sure the script is kept updated.

Collapsiblock maintainers recently decided to include a needed library in the module, making life easier for the end user:

As of 4.2.0 the slide-element library is now bundled into the built code. It is no longer necessary to mess around with CDNs or downloaded libraries.

From #3392947-8: Update readme to reflect JS library download no longer required → , see also https://git.drupalcode.org/project/collapsiblock#js-library.

For more arguments in favor of this approach, see also #3458611-9: Removing the polyfill-fastly.io library of webform.libraries.yml → .

@herved there is already npm-asset/js_cookie, which is usually used for third party libraries in Drupal. See https://www.drupal.org/docs/develop/using-composer/manage-dependencies#t... → .

But I am a big fan how this is solved in Photoswipe using hook_requirements(), together with 2 different library definitions (one using the cdn script the other the local script) and then create a settings page where you can enable using cdn (turned off by default).

Created an issue here: 📌 Module violates GDPR compliance through requring the js_cookie library via cdn Active . I think we should leave this issue open and adjust the description, once the other issue is fixed.

That's great @grevil, but what about the idea of pragmatically including the library in the module, making life easier for the end user?

@ressa, that would probably be the best approach for the end user I agree! But we would need to manually update the js library once in a while.

Although I am quite confused, as to why the npm-asset package and the gihub latest github release are version-vise out of sync! (3.0.5 vs. 1.0.0), seems like they point to two different repositiories. I'll comment that in the parent issue.

Thanks for being open to my suggestion! I left a comment in the other issue.

jan kellermann → made their first commit to this issue’s fork.

I like the approach of webform to use composer.libraries.json and created a new branch and merge request:

- Added composer.libraries.json
- Added js_cookie.install for requirements (see /admin/reports/status)
- Changed js_cookie.libraries.yml and js_cookie.module for a better overview
- Changed README.md from other fork.

I have changed js_cookie.libraries.yml to make it easier for developers to see that there are two different variants. In the previous js_cookie.libraries.yml this was not visible without looking at the code.

grevil → changed the visibility of the branch 3390616-support-local-library to hidden.

Reviewed. As written in the comment, I think a setting "Load library from CDN" explicitly might make sense to get rid of the warning, which otherwise makes sense for GDPR reasons.

For BC the CDN option could then be enabled by default. For new installs it could be disabled by default. That would be my favourite, what do you think @werk21?

Let's also document requiring "https://asset-packagist.org/package/npm-asset/js-cookie" as an alternative option!

I updated the MR and commented in MR directly.

@grevil Good idea. I added the command in the README.md.

Great work @jan kellermann! 🎉

Added a few comments, otherwise this looks good to go! 👍

Btw, I just worked with the composer merge plugin today in https://www.drupal.org/project/vuejs → , and I have to say, I am a bit underwhelmed! From your comment in 📌 Module violates GDPR compliance through requring the js_cookie library via cdn Active :

BTW: For the Bill of Materials software, an installation via Composer is preferable because the composer.lock files can then provide complete information about the software used, known security vulnerabilities and available updates. Therefore, a solution via Composer would be the better way than including the library file.

I thought, this issue would be solved through using the composer merge plugin! But unfortunately it doesn't, since it merely merges the provided "composer.libraries.json" with the main composer.json and since we define the library package ourselves, the maintainer of the module providing the composer.libraries.json now has to manually update the version, provide security information and update any information of the external library used! It is but a wrapper. So I am generally more in favor of the bower/asset-packagist approach, as the version automatically updates, if there is a new version upstream.

BUT, since this library wasn't updated for a while and probably won't get any updates any more anyway, it is perfect for this special use case 👍

I do still like the pragmatic approach of simply including the library in the module. It would have taken a few minutes, I believe ... just my 2 cents.

Thank you for feedback! I revised the README.md.

Both integration via composer.libraries.json and Asset Manager are used - both have advantages and disadvantages. If both are offered, users can choose their own way - as long as there is no “Drupal way”.

The PR does not apply to the 1.0.1 and dev versions of the module. Here is the patch for 1.0.1.

But it says !2 mergeable ...

Then the 1.x branch will be different from the 1.0.1.

Yes, they usually are different, since the dev branch is under development, and commits are being added continually ... Or do you mean something else?

Very nice @jan kellermann!

LGTM!

@dave reid: Could we please get this merged and release?
I just ran into 🐛 "cookies is undefined" Console error Active