Skip to content

Regression - Decorators #37068

Closed
Closed

Description

@bryanrideshark

Currently, we cannot upgrade to Angular 9.0 Ivy because of the regressions introduced by the Ivy compiler. If #35464 is not merged, we will have to stop upgrading Angular and leave it at version 9.0 indefinitely.

I am sharing this issue primarily so the Angular team can see how users are using Custom Decorators in the wild. I suspect I'm not the only one doing something like this.

Description

We develop an Nx workspace app, with multiple shared libraries powering both a Desktop, and Mobile application - responsive design doesn't cut it for our use case. In addition, we also have separate administrator views, and client libraries which are all generated from the same source code.

As such, dependency injection doesn't always meet our use case, even with the @Optonal decorator. Many of our components end up being shared between Mobile and Desktop, User and Administrator contexts, and as much as we can simplify their logic, we try to do so.

Currently, we use custom decorators to wire components in to pieces of the application which may, or may not be present. A great example is how we integrate with the map; In the Desktop application, a large on-screen map is available to most components. When users are entering addresses, they can click on the map, drag the pin around, etc. When on the Mobile application, this is not available.

The following code is how we connect the pin to the map in our application:

@Component({
    selector: 'app-foo',
    template: `
The origin pin is at {{orig?.lat}}, {{orig?.lng}}.
<button (click)="centerOnEmpireStateBuilding()">Center</button>
<button (click)="removePin()">Hide Pin</button>
`
})
// Wires up necesary parts, Object.defineProperty etc. on `ngOnInit`
@MapConnectedComponent()
export class FooComponent {
    
     // Actually connects this property to the map in a two-way databinding
    @MapConnectedAddress('orig')
    pin: {lat:number; lng: number;}

   centerOnEmpireStateBuilding() {
      // This will move the pin on the map to the appropriate location as well.
      this.pin = {lat: 40.7484, lng:-73.9857};
   }

  removePin() {
      // This will remove the pin
    this.pin = null;
  }
}

Because we have a small team of mostly junior developers, we try to focus on our developer experience heavily. Custom Decorators like this provide many benefits, chiefly among them:

  1. Declarative vs. Imperative programming
  2. Easy instruction for Jr developers - practically self documenting code.
  3. Free adherence to the Open / Closed principle, without need for inheritence.

Conclusion

#35464 is a blocker for us; We really don't want to unravel all the custom decorators we've developed (this is just one of the examples) just so that Angular can have a minuscule benefit in start up performance.

With Angular 8.0 I was able to achieve a very concise, easy to explain process. Before the decorators, each address which needed to be connected to the map, required 8 - 10 lines of code, not to mention injecting the right service, etc. Now, it's two decorators.

I hope this has been illuminating in how Metaprogramming is used in the wild, and helps you understand the user base better.

Activity

  1. bbarry commented on May 12, 2020

    @bbarry

    Somewhat off topic: Your type is wrong on the pin member and likely is going to cause issues in a more complex usage.

    It should be {lat:number; lng: number;} | null | undefined as those are valid states in that component. The fact that both null (explicitly set) and undefined (set because member is not initialized in a constructor) are valid on the same member leads me to believe that something is fishy with the design here.

    Anyway, how does https://github.com/ngneat/until-destroy work?

  2. added
    area: coreIssues related to the framework runtime
    regressionIndicates than the issue relates to something that worked in a previous version
    on May 13, 2020
  3. added this to the needsTriage milestone on May 13, 2020
  4. pkozlowski-opensource commented on May 15, 2020

    @pkozlowski-opensource
    Member

    @bryanrideshark you are right, this is the known issue with ivy version. We are tracking it already in #31495. Closing as a duplicate of #31495. It will be fixed by #35464

  5. bryanrideshark commented on May 15, 2020

    @bryanrideshark
    Author

    @pkozlowski-opensource , are we having to wait for Angular 10 for #35464 to be merged? Or Angular 11? Or can we expect this to land in a point release for Angular 9, say 9.2?

    I would love to be able to upgrade our application to Ivy - everything else worked, except for this.

  6. acn-masatadakurihara commented on May 20, 2020

    @acn-masatadakurihara

    Please adapt this fix to Angular9.

  7. angular-automatic-lock-bot commented on Jun 20, 2020

    @angular-automatic-lock-bot

    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.

  8. locked and limited conversation to collaborators on Jun 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: coreIssues related to the framework runtimeregressionIndicates than the issue relates to something that worked in a previous version

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions