rfcs
rfcs copied to clipboard
emberjs
Public router service
Thanks for writing this up! One question I have: is the idea that the router subclass could override all parent methods? Or should some of them be private (in TS terms and also in actual TS implementation)?
@chriskrycho as currently only methods listed as part of public API are actually implemented in RouterService, I assume we should keep all the methods as public and overridable as ember-engines would need to do those overrides.
is the idea that the router subclass could override all parent methods? Or should some of them be private (in TS terms and also in actual TS implementation)?
Confirm what @SergeAstapov said, currently there is no private API in the router service, and Ideally it stays that way (basically as more of an interface).
This has the practical effect that at least (for now) overridden methods should eventually call super in order to have actual routing things take effect, minimizing the potential for folks to get too ---creative--- with what they might do as the signature and intent will have to remain aligned.
I considered whether we should call this out in the RFC and opted against, mostly because it seemed to fall under "public APIs are public" and telling someone they must call super and must extend the base-class would artificially (and only pedantically) limit experimentation in this layer that is desired.
Re: "no private API"—I want to be clear here about two things:
-
I don’t have a vested interest in the particular decision we make here! I just want to make sure we make it carefully and with clear understanding of what we're committing to. 😄
-
Directly related: the fact that everything has been public so far has been uninteresting just insofar as it has not been subclass-able and therefore not overridable. Supporting subclasses overriding methods has very different semantic and maintenance implications than public call-ability. (This comes up in any kind of formal analysis of types, whether dynamic or static; ask me how I know. 😂) In particular, public call-ability does not have the same implications for upholding internal-only invariants that subclass-ability does.
Net, I think that it's worth being very explicit and intentional about this question and I would actually like to see it addressed explicitly in the RFC. I'd also like to see some actual analysis of the existing public API: do we actually want to say that it's fine for subclasses to override the definition of currentURL, for example? Or of location, or the timing and behavior of firing routeWillChange and routeDidChange events? Maybe, but that's not obvious to me!
Accordingly, I think it would be helpful to delineate, for each of these (perhaps at the category level if appropriate, but at a per-item level if that is more appropriate in other cases) whether it should be overridable by subclasses or not—
Methods:
Properties:
Events:
Bonus doh comment: it can’t actually use private in TS for the implementation details, because it is public for call-ability. But per both general good norms and per the SemVer for TS Types spec, it’s perfectly fine for us to define what the public API supports in terms of subclassing!
(FYI we didn't get to this agenda topic this week because #811 and #812 took up most of the time, will try again next week)
I don't have a ton of context on this, but maybe this comment from @ef4 will be important/useful to y'all since you have been thinking about it?
We had some back and forth in the Review meeting today around whether this should move to Exploring or not. There's definitely agreement that this is a problem we need to resolve, but that it's probably not going to be something that we can resolve until the new router design is more finalized.