compassus / compassus

A routing library for Om Next.
https://compassus.github.io/compassus/doc/1.0.0-alpha3/
Eclipse Public License 1.0
110 stars 18 forks source link

Support for passing route params to set-route! #2

Closed cmcfarlen closed 8 years ago

cmcfarlen commented 8 years ago

I'm evaluating compassus and its going mostly great! I would like to support routes like /item/42 but I ran into a snag. I can set query params in componentDidMount by asking bidi and pushy for the current route token and route-params, but if only the query param changes (i.e. path changes from /item/42 to /item/43) then the component doesn't get remounted.

Would it be possible to support this in compassus?

anmonteiro commented 8 years ago

Possible to do through other available constructs, not interested in supporting this at this time.

settinghead commented 8 years ago

+1. Would be really nice to have.

markhepburn commented 7 years ago

I'd be interested in seeing some docs about standard approaches for this, too. There's a dearth of information on queries for specific root objects (by id), strangely. All the examples tend to assume either a list at the root, or else don't use the id and assume the state already contains a single entity.

(Thanks for this project -- there really doesn't seem to be much consensus on handling routing in om-next otherwise!)

anmonteiro commented 7 years ago

@markhepburn have you seen these docs?

Let me know how they could be made better

markhepburn commented 7 years ago

Hi @anmonteiro , thanks for the response. I did see them; I think at the time I was trying to reconcile everything with our own setup, and the integration with the send function in particular. We use a standard REST api, augmented with a pull-api query parameter, so it should be a straight-forward case, but I was trying to figure out where exactly to insert the id. Basically all examples I've found tend to assume a simple case of a couple of routes, with no "details" views, and that all data is already in the app-db.

I'm assuming the top-level component would do something like (om/set-query! ui-detailsview {:params {:id 42}}), and that's then available to the send function in the query AST, but I haven't tried yet -- I'll do some experimenting.

Peeja commented 7 years ago

@markhepburn Here's the flow I think you're after:

  1. The URL changes, and you match the new URL to a new route (say, a route that shows a Widget, in this case #42), and you call (compassus/set-route! app :widget {:params {:route-params {:id 42}}}).

  2. Compassus merges that into the state, then re-reads the root component.

  3. The root component, now on the :widget page, has a query. That query asks for data that depends on the "current" Widget. At this point, it's up to you to decide how you want this to look, but one way to do it would be for your root query to look like:

    [{:current-widget
      [:widget/id :widget/name :widget/description]}]

Likely, the page component's query will be [{:current-widget (om/get-query Widget}], and it'll render a Widget, which will have a query of [:widget/id :widget/name :widget/description]. It should also have an ident of [:widget/by-id id], where id comes out of the props.

  1. Your parser now tries to read :current-widget. The implementation there finds the widget by the id and applies the joined query ([:widget/id :widget/name :widget/description]) to that widget, as

    (let [st @state
          id (get-in st [:route-params :id])
          widget (get-in st [:widget/by-id id])]
      (om/db->tree query widget st))

    There's an important snag here: if there's no data for the widget locally yet, not even an id, then the Widget component will receive a nil id and won't be able to form its ident, which will be a problem in a moment. To fix that, you can provide the id as a default in the get-in, since you know it:

    (let [st @state
          id (get-in st [:route-params :id])
          widget (get-in st [:widget/by-id id] {:widget/id id})]
      (om/db->tree query widget st))

    That forms the local part of the read.

  2. The remote part of the read turns all of that into an AST that can be sent to the server. For instance, you may want to transform it into (written as a query) [{[:widget/by-id 42] [:widget/id :widget/name :widget/description]}].

  3. The send function, critically, doesn't have access to the state, and doesn't know about your route params. Everything it needs to know needs to come in on the query it receives, which it now does. It fetches the name and the description of Widget #42 and passes that to the callback.

  4. Om merges the new Widget data into the app state, noting that the ident [:widget/by-id 42] now needs to be re-read. We made sure that the Widget component set its ident correctly, so Om's indexer finds that component and re-reads it. The local read happens a second time, but now all the data from the server is in the local state, and the parser returns it, populating the Widget component.

And voilà!


We're actually doing something slightly different in our app: our root query looks like:

[{:route-params
  [{:current-widget
    [:widget/id :widget/name :widget/description]}]}]

Then the routing system calls (compassus/set-route! app :widget {:params {:route-params {:current-widget [:widget/by-id 42]}}}), putting the ident in the app state instead of just the id. That means that the query above will "read through" the ident automatically with db->tree, so we don't need a special :current-widget key implementation (and an additional one for each entity type in the app). The downside here is that the routing code knows the name :widget/by-id (and, again, an additional one for each entity type in the app), which is a bit odd. I'm not convinced that what we've done is better than the version above.

markhepburn commented 7 years ago

@Peeja Thanks very much! That helps a lot; I've been scratching around in that general direction, but that definitely helps clear things up. I've noticed the issues around "pollution" by the different key names too, interesting that that doesn't seem to have a standard solution yet. Thanks again.