Bug 1750812 Comment 5 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

It looks like we may have a conflict between the groupby proposal and a library used by the site:

In the application code on line 38882:
```
arr.groupBy('name', function(name, group) {
        group = group.map('text');
        if(name === 'day') group = group.concat(set['weekdays']);
        set[name] = group.join('|');
      });
      set['modifiers'] = arr;
    }
```

the standard groupby does not take a string as it's first method, but a function. The engine throws at this point due to a typeError.

This may be due to the use of an outdated sugar.js library, which attempts to polyfill the array object with a groupby method: https://sugarjs.com/docs/#/Array/groupBy

The issue is in 1.1.2, the version the pager is using. It incorrectly skips polyfilling the array: 

```
  function extend(klass, instance, override, methods) {
    var extendee = instance ? klass.prototype : klass;
    storeMethods(klass, instance, methods);
    iterateOverObject(methods, function(name, method) {
      if(typeof override === 'function') {
        defineProperty(extendee, name, wrapNative(extendee[name], method, override));
      } else if(override === true || !extendee[name]) {
        defineProperty(extendee, name, method);
      }
      klass.SugarMethods[name] = { instance: instance, method: method };
    });
  }
```

In particular this line: `else if(override === true || !extendee[name])` -- the extendee (the array class) has defined the groupBy method. We end up skipping redefining it, and it causes the sugar library to break. 

This appears to be fixed in the most recent version. I don't know when it was fixed, because they appear to have had a major rewrite into typescript. 
however it appears to be fixed by 1.5: https://github.com/andrewplummer/Sugar/blob/0c6f63cc5e13f145a61a2211efef6e21f509578b/lib/core.js#L52-L60

Unfortunately, the new native version of this API does not have the same interface as the old sugar.js, which is why this is breaking. One solution here is to update the dependency pagerduty is using. 

Attached is a minimal example of the problem for testing.

Unfortunately, this isn't a bug in firefox's implementation, we just happen to be the first to implement the new featue and see the issue. This will require some discussion. It is a web compat/spec issue. For now, what I will do is add a flag to the feature while we try to determine how wide spread this issue is and whether it requires a rename of the method. You will be able to turn the flag off, and continue to use nightly for this page. In the meantime we will see how bad this breakage is.
It looks like we may have a conflict between the groupby proposal and a library used by the site:

In the application code on line 38882:
```
arr.groupBy('name', function(name, group) {
        group = group.map('text');
        if(name === 'day') group = group.concat(set['weekdays']);
        set[name] = group.join('|');
      });
      set['modifiers'] = arr;
    }
```

the standard groupby does not take a string as it's first method, but a function. The engine throws at this point due to a typeError.

This may be due to the use of an outdated sugar.js library, which attempts to polyfill the array object with a groupby method: https://sugarjs.com/docs/#/Array/groupBy

In 1.1.2 and other affected versions, it incorrectly skips polyfilling the array: 

```
  function extend(klass, instance, override, methods) {
    var extendee = instance ? klass.prototype : klass;
    storeMethods(klass, instance, methods);
    iterateOverObject(methods, function(name, method) {
      if(typeof override === 'function') {
        defineProperty(extendee, name, wrapNative(extendee[name], method, override));
      } else if(override === true || !extendee[name]) {
        defineProperty(extendee, name, method);
      }
      klass.SugarMethods[name] = { instance: instance, method: method };
    });
  }
```

In particular this line: `else if(override === true || !extendee[name])` -- the extendee (the array class) has defined the groupBy method. We end up skipping redefining it, and it causes the sugar library to break. 

This appears to be fixed in the most recent version. I don't know when it was fixed, because they appear to have had a major rewrite into typescript. 
however it appears to be fixed by 1.5: https://github.com/andrewplummer/Sugar/blob/0c6f63cc5e13f145a61a2211efef6e21f509578b/lib/core.js#L52-L60

Unfortunately, the new native version of this API does not have the same interface as the old sugar.js, which is why this is breaking. One solution here is to update the dependency pagerduty is using. 

Attached is a minimal example of the problem for testing.

Unfortunately, this isn't a bug in firefox's implementation, we just happen to be the first to implement the new featue and see the issue. This will require some discussion. It is a web compat/spec issue. For now, what I will do is add a flag to the feature while we try to determine how wide spread this issue is and whether it requires a rename of the method. You will be able to turn the flag off, and continue to use nightly for this page. In the meantime we will see how bad this breakage is.
It looks like we may have a webcompat issue: there is a conflict between the groupby proposal and a library used by the site:

In the application code on line 38882:
```
arr.groupBy('name', function(name, group) {
        group = group.map('text');
        if(name === 'day') group = group.concat(set['weekdays']);
        set[name] = group.join('|');
      });
      set['modifiers'] = arr;
    }
```

the standard groupby does not take a string as it's first method, but a function. The engine throws at this point due to a typeError.

This may be due to the use of an outdated sugar.js library, which attempts to polyfill the array object with a groupby method: https://sugarjs.com/docs/#/Array/groupBy

In 1.1.2 and other affected versions, it incorrectly skips polyfilling the array: 

```
  function extend(klass, instance, override, methods) {
    var extendee = instance ? klass.prototype : klass;
    storeMethods(klass, instance, methods);
    iterateOverObject(methods, function(name, method) {
      if(typeof override === 'function') {
        defineProperty(extendee, name, wrapNative(extendee[name], method, override));
      } else if(override === true || !extendee[name]) {
        defineProperty(extendee, name, method);
      }
      klass.SugarMethods[name] = { instance: instance, method: method };
    });
  }
```

In particular this line: `else if(override === true || !extendee[name])` -- the extendee (the array class) has defined the groupBy method. We end up skipping redefining it, and it causes the sugar library to break. 

This appears to be fixed in the most recent version. I don't know when it was fixed, because they appear to have had a major rewrite into typescript. 
however it appears to be fixed by 1.5: https://github.com/andrewplummer/Sugar/blob/0c6f63cc5e13f145a61a2211efef6e21f509578b/lib/core.js#L52-L60

Unfortunately, the new native version of this API does not have the same interface as the old sugar.js, which is why this is breaking. One solution here is to update the dependency pagerduty is using. 

Attached is a minimal example of the problem for testing.

Unfortunately, this isn't a bug in firefox's implementation, we just happen to be the first to implement the new featue and see the issue. This will require some discussion. It is a web compat/spec issue. For now, what I will do is add a flag to the feature while we try to determine how wide spread this issue is and whether it requires a rename of the method. You will be able to turn the flag off, and continue to use nightly for this page. In the meantime we will see how bad this breakage is.
It looks like we may have a webcompat issue: there is a conflict between the groupby proposal and a library used by the site:

In the application code on line 38882:
```
arr.groupBy('name', function(name, group) {
        group = group.map('text');
        if(name === 'day') group = group.concat(set['weekdays']);
        set[name] = group.join('|');
      });
      set['modifiers'] = arr;
    }
```

the standard groupby does not take a string as it's first method, but a function. The engine throws at this point due to a typeError.

This may be due to the use of an outdated sugar.js library, which attempts to polyfill the array object with a groupby method: https://sugarjs.com/docs/#/Array/groupBy

In 1.1.2 and other affected versions, it incorrectly skips polyfilling the array: 

```
  function extend(klass, instance, override, methods) {
    var extendee = instance ? klass.prototype : klass;
    storeMethods(klass, instance, methods);
    iterateOverObject(methods, function(name, method) {
      if(typeof override === 'function') {
        defineProperty(extendee, name, wrapNative(extendee[name], method, override));
      } else if(override === true || !extendee[name]) {
        defineProperty(extendee, name, method);
      }
      klass.SugarMethods[name] = { instance: instance, method: method };
    });
  }
```

In particular this line: `else if(override === true || !extendee[name])` -- the extendee (the array class) has defined the groupBy method. We end up skipping redefining it, and it causes the sugar library to break. 

This appears to be fixed in the most recent version. I don't know when exactly it was fixed, they had a major rewrite into typescript. 
In the previous version, it appears to be fixed by 1.5: https://github.com/andrewplummer/Sugar/blob/0c6f63cc5e13f145a61a2211efef6e21f509578b/lib/core.js#L52-L60

Unfortunately, the new native version of this API does not have the same interface as the old sugar.js, which is why this is breaking. One solution here is to update the dependency pagerduty is using. 

Attached is a minimal example of the problem for testing.

Unfortunately, this isn't a bug in firefox's implementation, we just happen to be the first to implement the new featue and see the issue. This will require some discussion. It is a web compat/spec issue. For now, what I will do is add a flag to the feature while we try to determine how wide spread this issue is and whether it requires a rename of the method. You will be able to turn the flag off, and continue to use nightly for this page. In the meantime we will see how bad this breakage is.

Back to Bug 1750812 Comment 5