Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor suggestions about @Input and implementing a custom decorator #767

Closed
lazarljubenovic opened this issue Nov 24, 2016 · 2 comments
Closed
Labels

Comments

@lazarljubenovic
Copy link
Collaborator

lazarljubenovic commented Nov 24, 2016

The code is currently using inputs array of strings on component metadata to tell Angular that properties with these names are to be considered inputs. This way of listing such properties is currently considered bad practice according to official Angular styleguide. It makes sense: maintaining is difficult because it's scattered around, and let's not even begin on typos and refactoring.

So my first suggestion is moving away from inputs and using @Input() instead.

In a similar fashion, we're maintaining an array of strings _mapOptionsAttributes in order to iterate over them more easily. This can be solved with a custom decorator; here's a proposed simple implementation.

function MapOptionAttribute() {
  return function (target: SebmGoogleMap, key: string) {
    if (!target.constructor._mapOptionsAttributes) {
      target.constructor._mapOptionsAttributes = [];
    }
    target.constructor._mapOptionsAttributes.push(key);
  }
}

And example usage:

class SebmGoogleMap {
   @Input()
   @MapOptionAttribute()
   public longitude: number;

   // We basically need this just for typing information in here.
   // The actual value is assigned through the decorator.
   private static _mapOptionsAttributes: string[];
}

The accompanying JS bin in action is also available.

How does it seem? Any suggestions? @SebastianM, if you like the idea, I could work out a PR.

lsnascimento pushed a commit to lsnascimento/angular2-google-maps that referenced this issue Dec 29, 2016
@lazarljubenovic
Copy link
Collaborator Author

It was mentioned in #863 (comment) that a reason for not doing this is being unable to generate docs. There doesn't seem to be a proper document generation tool for Angular (yet). However, ng-bootstrap has a custom script for generating docs which is probably worth checking out.

@stale
Copy link

stale bot commented Nov 14, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 14, 2018
@stale stale bot closed this as completed Nov 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant