Deprecate Injectable decorator factory #18604

Activity

1. vicb

   added

   area: coreIssues related to the framework runtimeIssues related to the framework runtime

   featureLabel used to distinguish feature request from other issuesLabel used to distinguish feature request from other issues

   on Aug 9, 2017
2. about-code commented on Aug 9, 2017

   

   about-code

   on Aug 9, 2017

   Last edited by about-code

   More actions

   @trotyl The reason may not make sense anymore, but well, there's a very strong argument in the quote why to not remove it.

   Although the HeroService doesn't have any dependencies at the moment, applying the @Injectable() decorator ​from the start ensures consistency and future-proofing.

   It reads like this:

   Hey write @Injectable even at places where you don't actually need it because its future-proof.

   So in parts we now invest twice into something which was useless from the start, but I admit its a search and replace task in case of the useless decorator.

   However going through tens or hundreds of constructors and copying their signature over to the respective deps array in a module is just very, very annoying. Migration tooling would be appreciated. In future when I write a constructor I need to write every argument type twice, once in the signature itself and once in the deps array.

   Without reflective injection Angular is loosing quite a lot of comfort (and even the ones who criticized Angular for the amount of boilerplate around NgModules could at least agree mostly that Dependency Injection wasn't that bad).

   Doubts about Reflect proposal?

   I wonder why reflective injection should become deprecated if Reflect has been proposed for standardization to the TC39 comittee? Is it really about the few KB of the reflect-metadata polyfill as stated in the commit message or is it because its unlikely the Reflect proposal will ever make it into stage 4?

   If its about a few KB to save I think deprecating reflective injection is short-sighted and won't add to the image of Angular.
3. about-code commented on Aug 9, 2017

   

   about-code

   on Aug 9, 2017

   More actions

   By the way I even feel a little schizophrenic to defend @Injectable because I never liked the fact of having a dependency on @angular/core in services which would otherwise be reusable without Angular. But since reflective injection was just about having some decorator for making TS generate the metadata I got used to writing

   import {BarService} from "./BarService";
   
   let Injectable = () => (target) => {};
   
   @Injectable()
   export class MyFooService {
       constructor(b: BarService) {
       }
   }

   where I absolutely didn't want a hard dependency onto Angular.
4. trotyl commented on Aug 10, 2017

   

   trotyl

   on Aug 10, 2017

   ContributorAuthor

   More actions

   In future when I write a constructor I need to write every argument type twice, once in the signature itself and once in the deps array.

   That's not true for AOT, it's already capable to not write @Injectable() if you only use AOT mode.

   And in Meeting Notes, Angular team already decide to make AOT the only option:

   Switching to make AOT be the only mode for many developers so they can test everything they used to use JIT mode for now in AOT

   So that problem won't exist anymore, just won't make JIT a first-class citizen.
5. about-code commented on Aug 10, 2017

   

   about-code

   on Aug 10, 2017

   Last edited by about-code

   More actions

   That's not true for AOT, it's already capable to not write @Injectable() if you only use AOT mode.

   Okay, its been a bit late last night. Thank's! Feel better this morning. 👍
6. Toxicable commented on Aug 10, 2017

   

   Toxicable

   on Aug 10, 2017

   More actions

   Just to reaffirm @trotyl point, here https://github.com/angular/angular/blob/master/CHANGELOG.md#500-beta3-2017-08-09
   It states:

   NOTE: This only applies to platform creation and providers for the JIT compiler. It does not apply to @component or @NgModule provides declarations.

   After showing how you now provide for those situations
7. maxkoretskyi commented on Aug 10, 2017

   

   maxkoretskyi

   on Aug 10, 2017

   More actions

   @trotyl , can you please elaborate on:

   And in Meeting Notes, Angular team already decide to make AOT the only option:

   What exactly does only option mean? We're using JIT compiler heavily in our large application alongside AOT. So I'm worried about making AOT the only option.
8. maxkoretskyi commented on Aug 10, 2017

   

   maxkoretskyi

   on Aug 10, 2017

   More actions

   And another question is how can @Injectable be removed if JIT compiler CompileMetadataResolver._getTypeMetadata depends on metadata for a class to resolve dependencies?
9. mhevery commented on Aug 10, 2017

   

   mhevery

   on Aug 10, 2017

   Contributor

   More actions

   @Injectable is still required for testing, hence it can't be removed. Tests do not go through AoT and hence depend on ReflectMetadata.
10. trotyl commented on Aug 10, 2017

    

    trotyl

    on Aug 10, 2017

    ContributorAuthor

    More actions

    Closing for now since it's not actionable yet.

    @mhevery , shouldn't allowing AOT in testing also being a part of Q3's OKR?
11. trotyl

    closed this as completedon Aug 10, 2017
12. angular-automatic-lock-bot commented on Sep 12, 2019

    

    angular-automatic-lock-botBot

    on Sep 12, 2019

    More actions

    This issue has been automatically locked due to inactivity.
    Please file a new issue if you are encountering a similar or related problem.

    Read more about our automatic conversation locking policy.

    _{This action has been performed automatically by a bot.}
13. angular-automatic-lock-bot

    locked and limited conversation to collaborators on Sep 12, 2019