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

added option to use generic for Transition.params #17

Merged
merged 2 commits into from
Dec 19, 2016
Merged

added option to use generic for Transition.params #17

merged 2 commits into from
Dec 19, 2016

Conversation

csvn
Copy link
Contributor

@csvn csvn commented Dec 19, 2016

Proposed fix for #16. This should not introduce any breaking changes, it should just give you the additional option of specifying which properties do appear in .params (with types).

Example in typescript playground.

@christopherthielen
Copy link
Member

I completely agree. Being limited to the array notation is frustrating. I've gotten used to params()['fooid'] though, since it seems to be idiomatic typescript.

I think it might be even more ergonomic to developers to simply return any.

trans.params().id

The downside is that you lose the ability to do compile-time checking like your example.

I propose changing the signature to:

   params(pathname: string = "to"): any;
   params<T>(pathname: string = "to"): T {
      return this._treeChanges[pathname].map(prop("paramValues")).reduce(mergeR, {});
    }

Omitting the type parameter gives you any

trans.params().id

but including a type parameter gives you a typed object

trans.params<SomeInterface>().asdfasdf // compile time error

@csvn
Copy link
Contributor Author

csvn commented Dec 19, 2016

Yeah, the usefulness of { [key: string: any } compared to just using any is a bit limited, but I was hesitant to change that for the initial commit in this pull request.

I like your suggestion, seems to cover any use cases I come up with. Pull request updated.

Interesting sidenote: Using the generic in the method signature (like your example) did not work. Swapping the order of the rows does work though (example in playground). Not sure if this is a bug or by design, but it seems weird to me.

Edit: fixed bugged link (share button works only once)

@csvn
Copy link
Contributor Author

csvn commented Dec 19, 2016

You can ignore the last part of my comment above. Seems I just misunderstood the type declaration when used in classes. I have used interfaces mostly when I needed multiple signatures.

class Test {
  foo(): number;
  foo<T>(): T { ... }
}
// is equivalent to:
interface Test {
  foo(): number;
}

@christopherthielen
Copy link
Member

^ That threw me for a loop too!

@christopherthielen christopherthielen merged commit eb12ec8 into ui-router:master Dec 19, 2016
@csvn csvn deleted the generic-param branch December 19, 2016 17:16
christopherthielen added a commit to angular-ui/ui-router that referenced this pull request Dec 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants