Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

false positive with: no-backbone-get-set-outside-model #123

Closed
bruno-garcia opened this issue May 7, 2016 · 2 comments
Closed

false positive with: no-backbone-get-set-outside-model #123

bruno-garcia opened this issue May 7, 2016 · 2 comments
Milestone

Comments

@bruno-garcia
Copy link

Using RouteParams from Angular2 will cause a false positive:

export class HeroDetailComponent implements OnInit {
    @Input()
    public hero: Hero;

    constructor(
        private heroService: HeroService,
        private routeParams: RouteParams) {
    }

    public ngOnInit(): void {
        let id: number = +this.routeParams.get('id');
        this.heroService.getHero(id).then((h: Hero) => this.hero = h);
    }

    public goBack(): void {
        window.history.back();
    }
}

The: routeParams.get('id') part will flag the rule: no-backbone-get-set-outside-model

@HamletDRC
Copy link
Member

Hi @bruno-garcia thanks for the information.

I think I am going to close this issue with 2 comments:

  1. The point of the no-backbone-get-set-outside-model rule is to make sure that you don't invoke dynamically dispatched methods that the compiler cannot enforce correctness on. For example, the compiler will not complain if you type route.params.get('id'), route.params.get('ID'), route.params.get('Id') but only one of those invocations will actually work at runtime. The design advice is to define a statically typed "getId(): number" method on the RouteParams object so the compiler can enforce these calls. So, in my opinion the rule actually has found an issue in your code that you should fix (but see my second point :) )
  2. The static type analyzer in tslint does not work, so it is impossible for a tslint to know if this.routeParams is a Backbone Model or if it is some Angular object. It tells us 'any' 99% of the time. This makes it impossible to know the type of routeParams, so we assume that any method that looks like a Backbone get is a Backbone get. tslint will fix this issue with this bug, but it is an architectural issue on their side: Lint Programs as a Whole palantir/tslint#680

How do you feel about this response? Do you think it is fair for me to close this item?

@bruno-garcia
Copy link
Author

Loud and clear. Hopefully palantir/tslint#680 will be done soon.
Until then I'll disable this rule.
Thanks,

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants