search

pbastowski / angular2-now

Angular 2 @Component syntax for Angular 1 apps
MIT License
145 stars 15 forks source link

Fixed a bug that prevented creating the same component twice #42

Closed dotansimha closed 8 years ago

dotansimha commented 8 years ago

@pbastowski

I removed it from the injectables, so in the next time it does not exists :(

Moving it to the end eliminates the need to remove them :)

pbastowski commented 8 years ago

@dotansimha I think not removing the appended services may crate a problem when a child class extends parent class. In such a case, parent injectables are appended to the end of the child's. So, if we don't remove them, I think there will be a problem. I need to think about this a bit more, but this thought just came to me, so, I wanted to put it out here.

dotansimha commented 8 years ago

@pbastowski You're right... The problem is that when we remove it, it does not exists on the next time that we create it. One other solution is to put it in the beginning of the $inject array, remove it before calling this.$$init.apply and then put it back... what do you think?

Thanks

pbastowski commented 8 years ago

I think removing $scope+$reactive before this.$$init.apply and putting them back after will work. Let's try it.

dotansimha commented 8 years ago

@pbastowski, thanks for you advice. Changed it, what do you think?

Thanks.

pbastowski commented 8 years ago

Let's try this. I'll merge and test on a project of mine to see if I get the expected results. Do you have a project that you can also test on?

pbastowski commented 8 years ago

@dotansimha @Urigo

I need your help with the testing of this new feature to

  1. ensure it works as advertised and
  2. confirm that it does not break existing apps that ran with previous versions of angular2-now.

This (closed) PR provides a couple of simple decorators on branch feature/defer-controller-3-decorate. However, for these decorators to work, a new version of angular2-now needs to be published, which includes significantly refactored way of instantiating component constructors. So, this is why thorough testing is required, before I publish these changes.

A user (tuvokki) has informed me that the angular-meteor socially tutorial already mentions and uses the new MeteorReactive decorator and that it does not work. Which is correct, because the version of angular2-now that supports is has not been published yet.

For my part, I can say that it appears to work as expected with my test app, but I would like to hear from you guys and if possible a few more people, with existing apps, that it also works for them.

Thanks for your help in this.

dotansimha commented 8 years ago

Hi @pbastowski @Urigo Made some tests, including creating a controller with Angular2-Now, and without it, with @Inject and without, also tried to extend classes - everything works great. Is there anything else you want me to test?

pbastowski commented 8 years ago

@dotansimha Thanks, that's the confirmation that I was after. @kamilkisiela has also written tests https://github.com/pbastowski/angular2-now/blob/master/tests/inject-spec.js#L98 that cover this topic. So, for now I think it's covered.