minification issues

[markuz-brasil]

First of all, I'm using 6to5 instead of traceur and mocha/chai/sinon instead of karma/jasmine. (and yeah, I ported the tests)

Flame war apart, the tests shouldn't fail. Which doesn't, unless it is minified. (And I didn't bother trying with minified traceur output...)

I pinpointed the issue to the class' context being somehow set to undefined in the minified form. So things like this.list fails because this is undefined.

Here is my project's git (minified) and all passing tests

I'm not clear where the issue actually is. I'm 99% certain that it isn't the testing framework. #93 doesn't fix it either. I'm left with this 6to5/6to5#343, a bug on uglify or on the di framework itself. (or some combination of them)

My work around is something like this: instead of writing this:

class UserList {}

class UserController {
  constructor(list) {
    this.list = list
  }
}
annotate(UserController, new Inject(UserList))

write this:

class UserList {}

class UserController {
  constructor(list) {
    this.list = list
  }
}

// add a factory around a class :/
annotate(UserFactory, new Inject(UserList))
function UserFactory (list) {
  return new UserController(list)
}

not ideal, but at least it gets the job done

[tusharmath]

@markuz-brasil How do you know this is not getting fixed with #93

[markuz-brasil]

I took a look at the code, copied and pasted. But I'm 100% convinced that the isClass and uglify are at fault.

I found out that uglifyy doesn't support Function.name link

I think that there is no way to distinguish a class from a function in ES5 (maybe ES6 too). So class Foo {} and function Foo(){} are exactly the same.

I feel like the solution is with annotations. Maybe a @Factory and isClasscheck for that with hasAnnotatiom(FnOrClass, Factory)

On Thursday, January 1, 2015, Tushar Mathur [email protected] wrote:

@markuz-brasil https://github.com/markuz-brasil How do you know this is not getting fixed with #93 https://github.com/angular/di.js/pull/93

— Reply to this email directly or view it on GitHub https://github.com/angular/di.js/issues/95#issuecomment-68486928.

[caitp]

I think that there is no way to distinguish a class from a function in ES5 (maybe ES6 too).

No, in ES6 they are deserialized as a class declaration or class expression (class BindingIdentifier ClassTail or class ClassTail)

[markuz-brasil]

Cool. Good to know. Thanks

On Thursday, January 1, 2015, Caitlin Potter [email protected] wrote:

I think that there is no way to distinguish a class from a function in ES5 (maybe ES6 too).

No, in ES6 they are deserialized as a class declaration or class expression

— Reply to this email directly or view it on GitHub https://github.com/angular/di.js/issues/95#issuecomment-68490746.

[tusharmath]

@markuz-brasil Actually #93 is a workaround so that the class doesn't throw up errors. In most real world scenarios every class would have some method. So if you update your code like this —

class UserController {
  constructor(list) {
    this.list = list
  }

  //Add a method to the prototype chain
  getFriends(){
    //In most cases you will always have a method.
  }
}

It should work.

[iammerrick]

Indeed, here is an example project: https://github.com/iammerrick/webpack-jsx-uglify-bug

[vojtajina]

There is no way to distinguish a class from a function, di.js uses the function/class name - if it starts with an uppercase letter, it's treated as a class. Sounds like the minifier changes class names into lowercase letters.

We can't treat everything as a class (call it with new operator), because if a non-class function returns a primitive value, this value wouldn't be used (the new object is used instead) and that's incorrect behavior.

We can add additional annotation such as @ClassProvider or @FactoryProvider.

Any other ideas?

[iammerrick]

I think the additional annotation is our best option.

[markuz-brasil]

I also agree.

Maybe something like @Constructor as a hint for using ClassProvider and the FactoryProvider as default way.

On Wednesday, January 7, 2015, Merrick Christensen [email protected] wrote:

I think the additional annotation is our best option.

— Reply to this email directly or view it on GitHub https://github.com/angular/di.js/issues/95#issuecomment-69069481.

[tusharmath]

Its just getting too verbose. Why can't we follow a convention as opposed to configuration?

Like any function with a name can be considered as a class otherwise its a factory.

[iammerrick]

This is also a problem with @InjectLazy.

[tusharmath]

Is there a drawback of using the annotation approach?

[iammerrick]

Addressing this with annotations here: https://github.com/angular/di.js/pull/96/files