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 10 EOL is close and this is used on > 1.000 projects.

Can someone please review the PR opened by valic?

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 📌 Replace Contextual Links BackboneJS usage with VanillaJS equivalent Needs work
# - https://www.drupal.org/project/drupal/issues/3204011 📌 Replace Tour BackboneJS usage with VanillaJS equivalent Needs work
# - 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 📌 Replace Contextual Links BackboneJS usage with VanillaJS equivalent Needs work
# - https://www.drupal.org/project/drupal/issues/3204011 📌 Replace Tour BackboneJS usage with VanillaJS equivalent Needs work
# - 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 📌 Drupal 10 support Needs work , 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 📌 Drupal 10 update Needs review .

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?)

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

Alex Bukach → made their first commit to this issue’s fork.

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 this issue 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 🐛 after upgrading to Commerce Core 8.x-2.36 this module doesn't work RTBC 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.

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.

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:

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 📌 Use PHP attributes instead of doctrine annotations Fixed

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

mglaman → closed merge request !11

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

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