Query parameters and path variables conflict with native DOM properties

[erikringsmuth]

Noted by @nomego in issue https://github.com/erikringsmuth/app-router/issues/81

Query paramaters like hidden=true overwrite the element's native DOM properties. For example, this URL hides the page: https://erikringsmuth.github.io/app-router/#/notes?hidden=true

The model generated by createModel() needs to change to look like this.

{
  params: {
    // path variables and query parameters
  },
  router // if bindRoute attribute is set on the router or route
}

This will be a breaking change and need to bump the version to 3.0.0. A PR is already open https://github.com/erikringsmuth/app-router/pull/90.

[tommie]

Another solution is having a whitelist of permitted parameters. Any reason not to go that route?

[erikringsmuth]

Checking a standard list of DOM properties could work. The downside to that would be if the user actually wanted to use a query parameter like hidden=true.

[tommie]

Good point. (Speaking of which, I'm also a bit concerned about W3C adding new attributes that suddenly get special meaning.)

Thanks for the explanation.

[nomego]

Let me add my thoughts to this case also.

First of all I've closed PR #90 since it depended on auto-binding template support, which won't be implemented. (Also issue #81 has been closed because of the same reason and because it turned into a discussion more than an issue.)

Benefits of scoped parameters:

• Performance (no need to loop over the model, just assign the whole model at once)
• Memory (no need to value-copy all properties, just reference the model)
• Declarative (templates will have {{ params.id }}, showing that it will use its value from path/query parameters)
• Less conflict risk (parameters conflicting with HTMLElement properties (which might change) or custom elements (which might have any property) - actually the property name params itself should probably just be a default but configurable by the user

Also if it were me, I wouldn't want to maintain black/whitelists of allowed/not allowed parameters/properties? And what will the documentation say? "Note, you cannot use these parameter names: id, hidden, ..." ?

Just want to make sure this change gets implemented in 3.0 :)

[tommie]

I agree overall.

Also if it were me, I wouldn't want to maintain black/whitelists of allowed/not allowed parameters/properties?

Coming from Polymer, I already maintain two sets of "allowed attributes" anyway (the attributes-attribute and the type-hinting Javascript property). So adding a third wouldn't really bother me (as it could be inferred from the attributes-attribute). But I realize other libraries may do it differently.

And what will the documentation say? "Note, you cannot use these parameter names: id, hidden, ..." ?

This is already a problem with attributes in HTML5 since you don't want to accidentally override global attributes (like title and hidden). Polymer has an [issue report[(https://github.com/Polymer/docs/issues/90) on this, but the docs are a bit vague, IMHO. Native properties are apparently even more of a problem.

Stating it in app-router would just make it more explicit.

[erikringsmuth]

I agree with @nomego on this one. One of the biggest things for me is keeping the API simple. It's easy to explain a params field vs. explaining a new whitelist attribute or why you need to avoid global attributes. It's a breaking change either way.