This is a small comment, I noticed while reading the code.

coffee.js is not a well closed library - but it is a simple fix.

Looking at the function definition defining the library 

(function ($, Drupal, drupalSettings, DrupalCoffee) {

4 inputs but DrupalCoffee is missing from the 3 variables passed in at the bottom.

It is a small thing but DrupalCoffee inside the library is pulled from the global state!

CommentFileSizeAuthor
params-0.patch306 bytesmartin107
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

martin107 created an issue. See original summary.

  • martin107 authored 4bf4a71 on 8.x-1.x 
    Issue #2728387 by martin107, michaelmol: coffee.js input parameters...
michaelmol’s picture

michaelmol CreditAttribution: michaelmol commented
Status: Needs review » Fixed

Thanks, i have commited your patch.

Status: Fixed » Closed (fixed)

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

Rajab Natshah’s picture

Rajab Natshah CreditAttribution: Rajab Natshah at Vardot commented

- Still we do have this:

 Uncaught ReferenceError: DrupalCoffee is not defined
    at coffee.js?v=8.2.4:229
martin107’s picture

martin107 CreditAttribution: martin107 commented
Related issues: +#2839064: Separate DrupalCoffee into a new file.

So this patch is a little old, perhaps we should revert.

Here is the logical contradiction

DrupalCoffee is passed into coffee.js as a dependency

DrupalCoffee is defined inside coffee.js

The next logical step is to define DrupalCoffee in a separate file and then specify the new library dependencies correctly

mattjones86’s picture

mattjones86
Location 🇬🇧 GMT+0
CreditAttribution: mattjones86 commented

Yeah currently the module throws a javascript exception due to DrupalCoffee not being defined in the global scope. This makes the module unusable in it's current state.

Perhaps that patch should be removed again until DrupalCoffee is split out into it's own file?

martin107’s picture

martin107 CreditAttribution: martin107 commented
Priority: Minor » Critical

This makes the module unusable in it's current state.

Just bumping to critical ... it is a pity I can't get reopen the status of this issue.

michaelmol’s picture

michaelmol CreditAttribution: michaelmol commented
Status: Closed (fixed) » Needs work

  • willzyx committed 705db67 on 8.x-1.x 
    Revert "Issue #2728387 by martin107, michaelmol: coffee.js input...
willzyx’s picture

willzyx CreditAttribution: willzyx commented

Reverted the patch

willzyx’s picture

willzyx CreditAttribution: willzyx commented
Priority: Critical » Normal
martin107’s picture

martin107 CreditAttribution: martin107 commented
Status: Needs work » Closed (outdated)

Thanks for reverting

I think this should be closed in favor of

#2839064: Separate DrupalCoffee into a new file.