Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($injector): workaround for MS Edge class detection #13697

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/auto/injector.js
Original file line number Diff line number Diff line change
Expand Up @@ -826,8 +826,10 @@ function createInjector(modulesToLoad, strictDi) {
if (msie <= 11) {
return false;
}
// Workaround for MS Edge.
// Check https://connect.microsoft.com/IE/Feedback/Details/2211653
return typeof func === 'function'
&& /^class\s/.test(Function.prototype.toString.call(func));
&& /^(?:class\s|constructor\()/.test(Function.prototype.toString.call(func));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think we need to test for constructor( only if we are on Edge, since it is possible that someone might have a function actually called constructor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgalfaso explained that:

(function constructor() {}).toString() === "function constructor() {}"

So this is not an issue

}

function invoke(fn, self, locals, serviceName) {
Expand Down
25 changes: 16 additions & 9 deletions test/auto/injectorSpec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

/* globals support: false */

describe('injector', function() {
var providers;
var injector;
Expand Down Expand Up @@ -247,42 +249,47 @@ describe('injector', function() {
});


// Only Chrome and Firefox support this syntax.
if (/chrome|firefox/i.test(navigator.userAgent)) {
describe('es6', function() {
/*jshint -W061 */
describe('es6', function() {
/*jshint -W061 */
if (support.ES6Function) {
// The functions are generated using `eval` as just having the ES6 syntax can break some browsers.
it('should be possible to annotate functions that are declared using ES6 syntax', function() {
expect(annotate(eval('({ fn(x) { return; } })').fn)).toEqual(['x']);
});
}


if (support.fatArrow) {
it('should create $inject for arrow functions', function() {
expect(annotate(eval('(a, b) => a'))).toEqual(['a', 'b']);
});
}


if (support.fatArrow) {
it('should create $inject for arrow functions with no parenthesis', function() {
expect(annotate(eval('a => a'))).toEqual(['a']);
});
}


if (support.fatArrow) {
it('should take args before first arrow', function() {
expect(annotate(eval('a => b => b'))).toEqual(['a']);
});
}

if (support.classes) {
it('should be possible to instantiate ES6 classes', function() {
// Only Chrome (not even the FF we use) supports ES6 classes.
if (!/chrome/i.test(navigator.userAgent)) return;
providers('a', function() { return 'a-value'; });
var clazz = eval('(class { constructor(a) { this.a = a; } aVal() { return this.a; } })');
var instance = injector.instantiate(clazz);
expect(instance).toEqual({a: 'a-value'});
expect(instance.aVal()).toEqual('a-value');
});
/*jshint +W061 */
});
}
}
/*jshint +W061 */
});


it('should publish annotate API', function() {
Expand Down
22 changes: 22 additions & 0 deletions test/helpers/testabilityPatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,28 @@ if (window._jQuery) _jQuery.event.special.change = undefined;

if (window.bindJQuery) bindJQuery();

var supportTests = {
classes: '(class {})',
fatArrow: 'a => a',
ES6Function: '({ fn(x) { return; } })'
};

var support = {};

for (var prop in supportTests) {
if (supportTests.hasOwnProperty(prop)) {
/*jshint -W061 */
try {
eval(supportTests[prop]);
support[prop] = true;
} catch (e) {
support[prop] = false;
}
/*jshint +W061 */
}
}


beforeEach(function() {

// all this stuff is not needed for module tests, where jqlite and publishExternalAPI and jqLite are not global vars
Expand Down