erikringsmuth / app-router

Router for Web Components
https://erikringsmuth.github.io/app-router/
MIT License
610 stars 83 forks source link

Two-way synchronization of attributes with location URL #98

Open tommie opened 9 years ago

tommie commented 9 years ago

I have a use-case where I want a query parameter to control the appearance of a dialog box. The same dialog can also be opened with a button. This is implemented as an attribute, with an observer opening the dialog.

It would be nice in terms of symmetry and allowing the user to easily copy-paste the shortcut URL if updated the window location URL in response to attributes changing. (I'm not sure if push or replace is appropriate, or how that would be configured.)

erikringsmuth commented 9 years ago

Having attribute changes automatically reflect back to the URL is useful but it gets tricky. The flatiron-director element supports this in a very rudamentary way with it's autoHash attribute.

These are some of the difficult things to tackle.

For now you can do this manually with history.replaceState(). Suppose you have a dialog attribute that controls if the dialog is open. Have your button toggle dialog, then modify your observer like this.

dialogChanged: function(oldVal, newVal) {
  history.replaceState(null, null, '#/new/url?dialog=' + newVal);
  // your existing observer code
}

This will update the URL but won't reload the page since history.replaceState() doesn't fire a popstate or hashchange event.

tommie commented 9 years ago

Knowing what part of the URL to change especially since the router can use hash paths and regular paths.

Is this ambiguous when parsing the path? I thought router just had a setting to choose hash or non-hash when parsing.

Does changing the URL reload the page or just change the URL for the next page load since the model is already updated?

The idea is to modify the URL to be consistent with the page's internal state. I'd be inclined to say route events should fire, but no re-instantiation of the elements. I don't think router's path patterns would allow a change to an attribute to cause another route to become active (as there is no filtering on attribute contents). If the active route could change, I would expect router to change routes as usual, to maintain overall consistency of the system (across URL, app-router's internal state and the instantiated element).

For now you can do this manually with history.replaceState().

+1. I'm planning on making this a generic thing managed by router's events for now, in preparation for a PR to app-router.

Would you oppose having autoHash in app-router itself (assuming the above issues get answered)?

erikringsmuth commented 9 years ago

I have a number of reservations on this. In general, auto updating the URL scares me. It could be done, but I'm going to be very critical about any code I pull in for this. I also think it's more of an edge case. I would never suggest this as the best practice for navigating in a web app. Things like controlling a modal window are the valid use cases.

There would be cases where a new instance of the page should be created so that domReady() and detached() lifecycle events are called, and there are cases where you wouldn't want a new instance and just update the single property. That could possibly be handled with the onUrlChange attribute https://erikringsmuth.github.io/app-router/#/api#onurlchange.

We would have to consider if changing the model in the before-data-binding would reflect back in the URL.

We would need object observers on both custom element and template instance models. Interestingly enough, this is an ES7 feature. I thought it was ES6 until today https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/observe.

flatiron-director can get away with this since it is a very simple router that operates on a single string. In app-router, I think history.replaceState() is the best way to handle things like modal windows since the user has full control of what is happening. I'd look at a PR for auto-updating, but I will be very hesitant to make it a supported feature.

tommie commented 9 years ago

Ok. I think the general guiding principle would be that the model params and the URL have a two-way synchronization. In Polymer, this feels (IMHO) more natural than only having a one-way sync. And I think in general the model params will not be modified by any code but app-router anyway. The exception being "special variant selectors" like a modal dialog box being controlled by a button, but also linked to a query parameter.

Maybe a reasonable invariant is simply that changing the value of a two-way parameter cannot cause a change to another app-route. That would certainly make things easier to reason about. If that's enough to make it easy to use and document, I don't know.

I'll implement the external version for my needs (since I have access to Polymer's observe.js), and if someone else asks for this, maybe that's a good time to pick up the discussion again.

erikringsmuth commented 9 years ago

Two-way binding is convenient but I think new frameworks will be moving away from two-way binding by default. The flux architecture in React doesn't do it at all. You use observers and explicitly propagate changes. Polymer is moving in this direction for 0.8 https://github.com/Polymer/polymer/blob/0.8-preview/PRIMER.md#property-change-notification-and-two-way-binding. It will now require a notify property to allow for two-way binding. Dynamic one-way binding will still happen down to children elements, but not up to parent elements by default.

The goal is to prevent children from mutating the parent unless it's explicitly allowed. I'd say the same goes for changing a URL. If the URL changes it makes sense to auto-propagate the changes to the page. That's how the router works today. I think it get's dangerous when changing the model automatically propagates back to the URL.

tommie commented 9 years ago

Eek, how sad. Thanks for pointing me to that doc. :)

On topic: What is the danger of propagating back, assuming that parsing/activating the new URL would make the model end up with the same bound attributes anyway?

erikringsmuth commented 9 years ago

It has to make assumptions about creating a new instance vs. updating the model. This would affect whether the custom element lifecycle callbacks are called like detached() and domReady(). It has to choose whether to call the router's events like activate-route-start, before-data-binding, etc.

Another note: Angular 2.0 is ditching two-way binding altogether.

tommie commented 9 years ago

Ack. That explains the different views. To me, it makes no sense to re-instantiate the custom element or fire events, because it's still the same route being active. So in my mind, there is no decision to be made. But that assumes two-way bindings is the norm for web components, which you've convinced me is/will be false.

Thinking some more about it, the reason I wanted this in app-router in the first place is mostly because I want the URL<->model mapping in a single place. More specifically the path pattern and possibly the set of allowed query string parameters. Having two opposing one-way sync mechanisms would definitely be a way forward, but the naive implementation there would essentially duplicate the path pattern, which doesn't feel as clean as having it in one place.