🇩🇰Denmark @googletorp

I've used this in the past and compared to the alternative (which is to fetch everything in one go) this is a major improvement. This is also an opt-in feature, so while making a lot of requests can cause problems, it's something the developers using this can determine if it's a problem or not. From what I understand if you're not using the feature we won't have any performance change.

Maybe it's better to get it in, solve a lot of people's problems and get real life feedback on which performance issues would make sense to address.

I think this is RTBC.

I wont go deep into this issue, but maybe worth for me to quickly touch on this.

I built the original version of the module (because I needed it) and did maintain it for about ~5 years with Centarro slowly taking over and owning the D8+ release fully.

In the many years I was active part of the drupal commerce community and worked on many drupal commerce projects I found that the main thing I was wishing for - was more modules to handle difference use cases like

- shipping
- discounts
- coupon codes
- admin reports

or maybe the module existing, but wasn't fully polished and/or difficult to use.

If adding advertisements can help fund modules which can strengthen the drupal commerce ecosystem, what's not to like? I get that people don't enjoy looking at ads in general, but building and maintaining modules is a tremendous amount of work. Unless you have tried it yourself, it's hard to understand how much work it requires. We need to look at ways of doing that in a sustainable way instead of fighting such initiatives.

Ryan didn't involve me in this decision but I fully support this direction and seeking partnerships which can help pay for maintenance of modules like this.

This has been fixed in 2.1.1

The patch at github seems to have a lot of issues.

Trying to execute the button I get: Uncaught CKEditorError: t.closest(...).querySelector(...) is null

When using same editor outside paragraphs you also get a similar error on load about querySelector which crashes editors if you have multiple on the same page. In general seems like checks are missing for the different selectors. Example

document.querySelector('XX').something

will cause JavaScript TypeError if XX is not present on the page, so you always need to check the output of querySelector and act accordingly if what you expected is not on the page.

I have created a test case which shows the failing issue and created a fix for the problem in the issue fork.

In general the problem is that before we did this:

foreach ($libraries as $library) {
  $with_deps = $this->getLibrariesWithDependencies([$library]);
  // We don't need library itself listed in the dependencies.
  $all_dependencies += array_diff($with_deps, [$library]);
}

The key problem is here:

$all_dependencies += array_diff($with_deps, [$library]);

I guess however wrote this assumes that this gives you the sum of the two arrays, but that is not the case, this only add the dependencies that exceeds the amount of dependencies we have of the left have side. The test case illustrates this.

googletorp → created an issue.

Rerolled the patch from #8 by manual comparing what was required.

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