stencil-community / stencil-router

A simple router for Stencil apps and sites
https://stenciljs.com/
MIT License
189 stars 55 forks source link

Feature Request: Map match.params.* to Props #15

Open CrshOverride opened 7 years ago

CrshOverride commented 7 years ago

Currently, using Route parameters in a Component is clunky. Consider the following Component:

@Component({
  tag: 'my-hello'
})
export class MyHello {
  @Prop() name: string;

  render() {
    return (
      <span>Hello, {this.name}!</span>
    );
  }
}

If you want this component to used as a route, you currently have to modify it by adding a Prop to handle the route match as well as convert name to State so that the value can be updated internally via the match:

@Component({
  tag: 'my-hello'
})
export class MyHello {
  @Prop() match: any;

  @State() name: string;

  componentWillLoad() {
    if (!!this.match.params.name) {
      this.name = this.match.params.name;
    }
  }

  render() {
    return (
      <span>Hello, {this.name}!</span>
    );
  }
}

Any routable component must now:

  1. Be aware that it could be routable and accept a match.
  2. Make any route params State instead.
  3. Be responsible for optionally updating their state based on their match (and know exactly what route param identifiers they allow).

Obviously this is less than ideal. If I want my components to be truly re-usable, they shouldn't be bound to the routing mechanism.

Request

If param names in the route match Props on a component, they should be automatically mapped upon instantiating the component. This places the burden of mapping names on the consumer of the component instead of the author as well as allow easy re-use of un-modified components.

jthoms1 commented 7 years ago

I would say that if you are using a router the components being routed to should be knowledgable about what information they are receiving via router. Another approach could be to use routeRender prop on the route component. It accepts a function that takes props which You can then set a function to map the match results to props of another component.

Here is a simple example. https://github.com/ionic-team/stencil-router/blob/master/src/components/__tests__/test-app.tsx#L47

Also you probably should not update state based on match. It is a prop so just make use of that within your render function.

cjorasch commented 7 years ago

+1 for mapping to properties.

This already happens with componentProps and seems like a pretty standard pattern for what developers would end up doing anyway. Even the Stencil website says "you can define parameters in the url /foo/:bar where bar would be available in incoming props" which implies this approach.

The approach using the special match property seems a bit awkward. It adds to the list of "special" property names that have meaning without being part of any strongly typed interface. Somehow developers need to know that they should not make their own alternate use of "match" and "history" (because of their use by the router), "color" and "mode" (because of their use in Stencil), "id" and "title" (because of their use in HTMLElement), etc. It seems like something more along the lines of connect or context might help reduce conflicts.

components being routed to should be knowledgable about what information they are receiving via router

It is useful to be able to get this information in some cases but it seems like most use cases would just copy the value like in the example above. A component transforms a set of input properties into rendered output. If you set a property via an html attribute, JSX binding, componentProps, url segments, element property changes, etc. it seems like the component should generally have the same output.

Also you probably should not update state based on match. It is a prop so just make use of that within your render function.

There are times where you still might want to use State() if the match approach is used. The Ionic sample apps use this approach.

jthoms1 commented 6 years ago

I have decided not to implement this feature. I think its more important to keep url props independent.

jthoms1 commented 6 years ago

After more thought I am going to reopen this. I don't want this to work by default, but I think an extra param to the route component could allow props to be passed on.