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

Issues with current Route definitions #48

Closed
chriskrycho opened this issue Sep 18, 2017 · 4 comments
Closed

Issues with current Route definitions #48

chriskrycho opened this issue Sep 18, 2017 · 4 comments
Labels

Comments

@chriskrycho
Copy link
Member

Using the latest, I bumped into the following issues:

  • beforeModel and afterModel (a) should be generic and (b) are overly strict. Should probably be something like this:

    beforeModel<T>(transition: Transition): Rsvp.Promise<T>;
    beforeModel(transition: Transition): void;
  • Similarly, model should probably be something like

    model<T>(params: {}, transition: Transition): T | Rsvp.Promise<T>;
    model(params: {}, transition: Transition): void;

I also ran into issues with the ActionsHash bits combined with the user of Route.extend(SomeMixin), but I strongly suspect the mixin pattern is to blame there, rather than the Route definition.

@dwickern
Copy link
Collaborator

The actual return type would be something like Promise<any> | any | void, which reduces to any

@dwickern
Copy link
Collaborator

ActionsHash should be fixed in #24

@chriskrycho
Copy link
Member Author

🤔 Is there anywhere you'd directly call the hook yourself? I guess not, thinking about it. I was thinking it'd be helpful to be able to define those to work in a specific way so that you'd know whether you were returning what you'd claim. I think if it returns any, the override a user supplies should be able to be more specific, right?

@dwickern
Copy link
Collaborator

Right, you could have a more specific return type annotation to check that you implemented it correctly.

In practice all my model hooks would return something like DS.Model | PromiseLike<DS.Model>, however we can't assume everyone uses E-D

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

No branches or pull requests

2 participants