Document using local library file, with CDN as fallback (Module violates GDPR compliance)

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

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

@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....

UPDATE: js-cookie/js-cookie is also abandoned:
This package is abandoned and no longer maintained. The author suggests using the npm-asset/js-cookie package instead.
So we should definitly document the npm-asset library instead.

UPDATE 2: Wrong js-cookies...

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: #3471176: Module violates GDPR compliance through requring the js_cookie library via cdn . I think we should leave this issue open and adjust the description, once the other issue is fixed.

@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.

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.

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 #3471176: Module violates GDPR compliance through requring the js_cookie library via cdn :

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 👍

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.

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

Very nice @jan kellermann!

LGTM!

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

Please release this !

I am fairly certain @dave reid is either super busy, or not working with Drupal anymore.

Over the last year or so, I tried to contact him via Slack + Mail multiple times regarding #3435760: Automated Drupal 11 compatibility fixes for xmlsitemap, to no avail.

I guess somebody has to create an issue regarding module maintainer ship, to get this issue merged.

I suggest forking this project, it seems like the good developers are leaving Drupal, and for a good reason.

I opened #3515677: Offering to maintain JS Cookie to push things forward here. No reply from dave reid yet.

Thank you @anybody

I'm still here and am working to review this. I'm not sure we need the logic to assume local by default, then CDN, rather than the other way around, since they're functionally the same?

Thanks dave, that's great! :)
I think the logic behind was "privacy first" and I like it. But of course you could also argue "usability first". It shouldn't be the focus or a blocker.

This patch is working well, would it be possible to have a new release including this as is please?

I would also prefer the local library for privacy reasons.

Maybe someone could ping @dave-reid? Is he active? Otherwise maybe request (co-)maintainership?
This project is very widely used!

Let's request new (co-)maintainership @anybody it seems like the maintainer is not interested or busy to review this project or something.
With all due respect of course.

I re-opened the issue #3515677 and suggested giving anybody maintainer privileges.

Thanks @jan kellermann! Would be great if you could also assist as maintainer! :)

@anybody I would be happy to assist you :-)

I would like co-maintainer too, if you don't mind

Merged! Thank you all so much!

@jan kellermann feel free to tag a new stable release, if you agree to.