Drupal 10 support

Removed tag that should only be used for issues created by the update bot.

Opened MR, added additional changes on top of patch from #3 - mostly removal of jQuery once in favor of core once implementation

Drupal 9 EOL is close and this is used on > 1.000 projects.

Sure thing!

"getPath()" already returns a relative path as a string:

@return string — The Drupal-root-relative path to the specified extension.

Drupal < 9.5 has reached its EOL.

Notes on using "internal.backbone":

internal.backbone:
# Internal library. Do not depend on it outside core nor add new core usage.
# The library will be removed as soon as the following issues are fixed:
# - https://www.drupal.org/project/drupal/issues/3203920
# - https://www.drupal.org/project/drupal/issues/3204011
# - https://www.drupal.org/project/drupal/issues/3204015

Same with "internal.underscore":

internal.underscore:
# Internal library. Do not depend on it outside core nor add new core usage.
# The library will be removed as soon as the following issues are fixed:
# - https://www.drupal.org/project/drupal/issues/3270395
# - https://www.drupal.org/project/drupal/issues/3203920
# - https://www.drupal.org/project/drupal/issues/3204011
# - https://www.drupal.org/project/drupal/issues/3204015

"once" calls have "find" already integrated in their specification, see https://www.drupal.org/node/3158256. Furthermore, it was possible to drop the jquery dependency for the "add-to-cart" library.

Hey all,
I have update the module to be worked with drupal 10 & created the patch also.
Please review & let me know if any other issues.

Concerning #13, here is a pretty good guide on removing "underscore" with vanilla js: https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore. And removing backbone shouldn't be too hard.

As Drupal 10 also introduces native es6, see https://www.drupal.org/node/3305487, I suggest removing the old non es6 files and replacing them with the es6 files.
A new SemVer branch is adviced.

Just discussed #13 internally with @Anybody, and it probably makes sense to depend on the internal library for now and create a major follow-up issue removing the deprecated backbone and underscore dependencies.

The "commerce_cart_api" module, this module depends on, is not yet Drupal 10 ready.

POSTPONED on #3332548: Drupal 10 update.

Seems to work as expected, we now only need to wait for the parent issue to resolve.

Let's review this now, so we can push this forward asap!

Thanks @Grevil, great work! All reviewed and LGTM!

Now let's hope the parent task also gets resolved and add a major follow-up to replace the internal libraries!
(Perhaps AI can help to do that?)

@Maintainers: Please update credits!

I don't think @mglaman wants this to not be backwards compatible, so we should give it a similar treatment like in #3332548: Drupal 10 update.

Adding in the fork, I am getting an empty div instead of my cart block

<div class="cart-flyout" data-once="cart-block-render"></div>

Am I missing a library somewhere perhaps?

Adding in the fork, I am getting an empty div instead of my cart block

I am experiencing the same, although this used to work earlier. Still trying to figure it out.

I am afraid that somewhere in Grevils changes the js has gotten corrupted.

When I patch with the first commit made in the merge request, my cart block works as expected.
You can use this patch link for composer patches: https://git.drupalcode.org/project/commerce_cart_flyout/-/commit/9cca30d...

Since the last commit from AlexBukach is not in there, you will have to use the patch from #3364476: after upgrading to Commerce Core 8.x-2.36 this module doesn't work which fixes the same thing. (That is what I did.) You could probably isolate the last commit from this MR in a similar fashion as I isolated the first commit too.

Edit, this is the patch of the last commit:
https://git.drupalcode.org/project/commerce_cart_flyout/-/commit/2d1f81e...

Adding related issue with overlapping patch.

Having previously got this working on one site, I now can't get it to work on another. It would be really great to get these changes bundled up into a release.

Raise Minimum Commerce compatability to 2.36

composer.json has a "drupal/commerce" of ^2.0 - now that the constructor signature has changed, the minimum version of commerce this module will work with is 2.36.

Use FileUrlGenerator for icon.

Without using FileUrlGenerator for setting the icon path, it does not include the leading "/" (or leading base path) and also cannot be altered for serving from CDN.

Use jQuery syntax for once call in module.js

This was the source of the cart block not loading for me. The use of plain once() call inside a jQuery context did setup the flyout but would not populate the contents of the cart block, despite marking it as being run once.

Thanks ELC, I believe the block is showing again.
Leaving at NR because of #24.

Backwards compatibility is actually pretty much out the window right now. Despite the current info.yml claim of ^9.5 || ^10, this fork should be a hard ^10, and given the cut-off, a new Semantic Version based branch/release should be made.

1. 0c5f7bf2 - Rename ".es6.js" files to ".js" and remove old non es 6 js files; This a hard ^10 dependency, unless the JS just happens to be ES5 compatible: https://www.drupal.org/node/3305487
   This does really seem to be more a browser thing than a drupal core restriction.
2. Min drupal/commerce:^2.36 due to the constructor argument change; commerce is "drupal/core": "^9.3 || ^10"

Might have to separate out only was is absolutely necessary for D10 compat into 8.x-1.x branch, and then make a 2.0.x branch for the version with the hard breaking changes.

Backing out the es6 change, min version can be core:9.3.0. Not actually sure were the min version 9.5 has come from.

How backware compatible does it need to be given the issues?

@ELC I haven't looked at the all the code / progress, but what does the dropping of backwards compatibility imply?
Is it just that the module will not be able to work on both D9 and D10 simultaneously?
Or is it that there will be no upgrade path when upgrading from D9 to D10?

If it's only the first, I would opt for a D10-only branch (2.x?) and not worry about any D8 / D9 backwards compatibility there.
D8 and D9 are no longer supported, and they already have a working 8.x-1.9 release, so websites staying on those Drupal core versions can stay with that release. But I think the majority of sites will upgrade to D10, and thus to the 2.x branch of this module.
This module is currently the only remaining blocker for one of our sites, so I'd like to assess when we will be able to unblock this.

The https://www.drupal.org/project/commerce_cart_api module now has a D10 release, so that's no longer a blocking factor in getting this module to D10 I reckon?

I probably should have set this to NR as it needs @mglaman or another maintainer to decide the path to choose.

It boils down to 1 release branch or two. I think it should be done as 2 releases for ease of upgrading:

8.x-1.10 release: ^9.3 || ^10

• Add the es6 files back in, fix up js as needed.
• Minimum drupal/composer: ^2.36
• Mark branch as not-recommended
• Final release for this branch

2.0.0 release: ^10

• Use the MR as is.
• Minimum drupal/composer: ^2.36
• Recommended branch

Any older sites then have a progression to the fully supported version.

Choosing the path to a single release branch means jumping straight to D10 only. Or just leaving the es6 compiling in until its use is removed which would allow D9.3/D10.

A new release branch will be needed at some point as the Block Annotation will be changed to an Attribute in D10.2 which is again a hard cut-off to previous versions. Again which could be delayed until D11 although there will be a deprecation notice. @see #3252386: Use PHP attributes instead of doctrine annotations

Can someone explain why we had to change the files from es6 extension? We should just fix usage of `once` in the JavaScript. This makes it easier to review.

We should be able to do a minor release and support D10 with minimum changes.

Also, the event subscriber could use a decorator and setter pattern to avoid constructor breaks. But that can be a different issue.

@mglaman, it looks like sarwan_verma did that back in comment 15 - https://www.drupal.org/files/issues/2023-09-28/commerce_cart_flyout-3359...

I was able to use the patch with https://github.com/mglaman/composer-drupal-lenient (wish I found this project earlier...). I'm kicking the tires of commerce (no live site yet), but things work.

@mglaman can you review if this is the correct direction, and then maybe someone with more commerce experience than I can polish the patch (if needed). Thanks!

I made a new branch, 3359110-d10-minimal-change, that combines @sarwan_verma's work from #15 and @AlexBukach's from #26. This working for me on both D9 and D10.

Thanks @rwanth I'll find some time to review. I appreciate it!

This looks great, thank you! https://git.drupalcode.org/project/commerce_cart_flyout/-/merge_requests/13

Works great on my side, just on /admin/reports/updates it shows as "Not compatible".

@mglaman any plans for a release? Currently there's no more D10 compatible release visible on the project page!

Apologies for the repeated question here but are there plans for a release of a D10-compatable version of this module? We'd like to implement this on a D10 site.

Thank you for your work on upgrading this so far.

Taking a look and landing this

Updating credit. Thanks, everyone. Added GitLab CI in another issue. Waiting to see those results (no tests, but PHPStan will run.)

Thank you, everyone, and for your patience! Cutting a release shortly.