feat(typescript): declare View, Lifecycle, NavigationTrigger

[platosha]

Fixes #422

---

This change is 

[claassistantio]

All committers have signed the CLA.

[abdonrd]

How should works? Like this?

@customElement('page-home')
export class PageHome extends LitElement implements VaadinRouterView {
  // ...
}

[platosha]

@abdonrd for this.location there is a TS class you could merge with or cast into:

import {Router} from '@vaadin/router';

class MyView extends HTMLElement {
  connectedCallback() {
    this.location;
  }
}
interface MyView extends Router.View {}

or

class MyView extends HTMLElement {
  connectedCallback() {
    (this as Router.View).location;
  }
}

for lifecycle there is an interface:

class MyViewWithBeforeEnter extends HTMLElement implements Router.Lifecycle {
  onBeforeEnter(
      location: Router.Location,
      commands: Router.PreventAndRedirectCommands,
      router: Router
  ) {
    location.pathname;
    return commands.redirect(router.urlForName('home'));
  }
}

[abdonrd]

Thanks for the info, @platosha!

And if I need both? I need to merge + interface?

@customElement('page-home')
export class PageHome extends LitElement implements Router.Lifecycle {
  // ...
}
interface PageHome extends Router.View {}

[platosha]

@abdonrd yes, you can combine them as needed. Note, that in lifecycle callbacks you could use location argument in place of this.location.

[abdonrd]

I'm not a TS expert, so, why this difference?

[platosha]

Essentially there are two opposite ways of interaction between the view component and the Router:

• lifecycle callbacks are optionally provided by the component for the Router to consume. So this is pretty straightforward, there is a TypeScript interface Router.Lifecycle with some optional callbacks. When your component implements this interface, that requires the component’s class to have an implementation compatible with the interface. Then you also need to implement (add to your class) some of callbacks to meet this requirement.
• the this.location property is provided by the Router for the component to consume. This is a tricky case: classes, basically, can either use properties they implement, or the inherited ones, meaning that they are implemented higher in the hierarchy. this.location is neither, instead it is added in runtime by the Router. Implementing an interface is not appropriate here: the class should not be required to implement a property, the implementation is provided already by the Router. But because the implementation is runtime, TypeScript is not aware of it. So here need to cover exactly that: somehow indicate for TypeScript, that apart from properties declared in the class and inherited from LitElement, the class instances also have an additional property coming from the Router. This is what we do here:

class MyView extends HTMLElement {
  connectedCallback() {
    this.location;
  }
}
interface MyView extends Router.View {}

In the PR, Router.View is a class definition, not an interface, containing location property. Unlike with interfaces, this is aimed not to require implementation somewhere, but rather to indicate an existing implementation of the property.

Then, we need to declare that MyView extends from multiple parents, in a way, both from LitElement / HTMLElement parent, and the Router.View. While literally extending multiple parents is not possible, we could have a similar result using declaration merging: class MyView {} interface MyView {} pair is merged into same thing by TypeScript compiler. Here the interface keyword is not to be read literally, but it’s rather a TypeScript syntax trick to append more declarations to the class above from the Router.View.

I have to apologise, I understand that this usage might feel a bit awkward. This comes down to the fact that the Router runtime adds a property to the component, and from the component’s author standpoint the location is coming from seemingly nowhere: neither form the class nor from the hierarchy. This violates design principles and should definitely be reconsidered in future API revisions, I think. For now, without introducing changes to runtime or API, that’s about all what we could do.

[abdonrd]

I have understood! Thank you very much for the detailed explanation, @platosha!

[haijian-vaadin]

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @platosha)

---

interfaces.d.ts, line 143 at r1 (raw file):

Lifecycle

Instead of having one Lifecycle interface, would it make sense to align with Flow navigation APIs? i.e. have 3 interfaces BeforeLeaveObserver, BeforeEnterObserver and AfterNavigationObserver?

[platosha]

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @haijian-vaadin)

---

interfaces.d.ts, line 143 at r1 (raw file):

Previously, haijian-vaadin (Haijian Wang) wrote…

Lifecycle

Instead of having one Lifecycle interface, would it make sense to align with Flow navigation APIs? i.e. have 3 interfaces BeforeLeaveObserver, BeforeEnterObserver and AfterNavigationObserver?

Done. Also, now that I’ve discovered how to do that, exported new types as module-level, so that users don’t have to use Router.Something namespace with them.

[manolo]

Reviewed 3 of 5 files at r1, 2 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @haijian-vaadin)

[platosha]

Hm-m, maybe the weird View class is unnecessary.

Now after trying using it with LitElement, I realise that you might actually want to declare location in your element class — to make LitElement observe its changes.

So, exporting just the RouterLocation type seems more useful.

Then, it could be either declared as optional:

import {RouterLocation} from '@vaadin/router';

@customElement('my-view')
class MyView extends LitElement {
  @property({type: Object}) location?: RouterLocation;

  render() {
    return html`Current path: ${this.location ? this.location.pathname : ''}`;
  }
}

Or, could take the initial value from the router instance, that way it would be always present:

import {router} from './router';

@customElement('my-view')
class MyView extends LitElement {
  @property({type: Object}) location = router.location;

  render() {
    return html`Current path: ${this.location.pathname}`;
  }
}

@abdonrd any thoughts?

[abdonrd]

I honestly imagined something like:

export class PageElement extends RouterView(LitElement) {
  // ...
}

And then create the pages like:

@customElement('page-home')
export class PageHome extends PageElement {
  // ...
}

@customElement('page-about')
export class PageAbout extends PageElement {
  // ...
}

Because really all pages have access to location and all callbacks.

But it's true that this logic is given in runtime, and not by the mixin itself.

[manolo]

Reviewed 2 of 2 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @haijian-vaadin)

[platosha]

Same changes were merged from #427