Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Routing #15

Merged
merged 7 commits into from Feb 6, 2015
Merged

Routing #15

merged 7 commits into from Feb 6, 2015

Conversation

refractalize
Copy link
Member

Run the example:

watchify examples/routing.js -v -o examples/public/app.js
node examples/server.js

Then hit http://localhost:4000/

You'll notice that there is a 300ms delay when loading the /objects and /objects/:id pages. This is to simulate an AJAX load time. However, when navigating forward and back we use history.state so there is no delay. When we refresh the page (F5, ⌘-R), we fetch new state instead of using history.state.

@refractalize
Copy link
Member Author

I think we can write a function like router.respond(req, res, render, model).

var render = require('../browser/app').render;
var router = require('plastiq/router');

app.get('*', function (req, res) {
  router.respond(req, res, render, { user: req.user });
}

@joshski
Copy link
Member

joshski commented Jan 16, 2015

Yeah I like where this is going. I guess we can use an iframe to get karma tests round it.

@refractalize
Copy link
Member Author

Yeah good point. I was thinking of firing up selenium but if we can do it with an iframe then much better.

@refractalize
Copy link
Member Author

Just did a test with Karma. Tests are already run inside an iframe and pushState and popstate work perfectly.

h('h1', 'objects'),
h('ul',
objects.map(function (o) {
return router.link('/objects/' + o.id, h('li', o.name));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

router.link() generates an anchor tag, so I guess you meant to nest the link inside the h('li').

I wonder if regular anchors (as in h('a', ...)) should automatically plug into the router. That seems like it would be a good reason for routing to be integrated into plastiq itself. The usual reason for using something like router.link instead of regular anchors is to plug into some "named routes" system, e.g.

router.link('showObject', { id = o.id }, o.name)

Personally I like named routes on bigger projects because: 1) the framework can throw an error when you link to bad URLs instead of when you follow those links and 2) your code is isolated from changes to the URL.

However mandatory route names complicate route setup code. Perhaps a middle ground is that routes could have name == pattern by default. That would allow this:

router.link('/objects/:id', { id: o.id }, o.name)

...or even this:

router.link('/objects/:id', o.id, o.name)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I like having validated routes for link, and I think having the pattern there is more "real". You can always make a function like showObject(o.id) => "/objects/" + o.id, if it bothers you.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking of refactoring from a regular anchor, another option:

h('a', { href: ['/objects/:id', o.id] }, ...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember being quite confused when angular automatically hijacked all my anchor tags. I wasn't happy about it :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree, too surprising, annoying in some cases.

But I do like the idea of routing anchors looking pretty similar to regular anchors. This may be less funky:

h('a', { route: ['/objects/:id', o.id] }, ...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's it.

@refractalize
Copy link
Member Author

So, we have a route attribute, and we apply that to any element, basically overriding the onclick event to cause a route change. This would include buttons, divs and all manner of clickable things...

@joshski
Copy link
Member

joshski commented Jan 16, 2015

Sounds good to me. When you say "overriding", does that mean "cancelling any existing onclick"? I was wondering about this with the binding attribute too...

@refractalize
Copy link
Member Author

yeah, in the binding case, I execute the binding event handler before the user event handler, and return the result of the user handler. See https://github.com/featurist/plastiq/blob/master/rendering.js#L89-L103. Seems to work.

In the case of the onclick here, especially with anchors, is that we would have to ev.preventDefault().

@refractalize
Copy link
Member Author

Some changes to the API:

  • The router is no longer built-in, it's in a separate module: plastiq/router. This may change, but for the moment I think the router needs to prove itself before being in the core.
  • Links are made with h('a', {href: '/path', onclick: router.push}, 'my link'), not h('a', {route: '/path'}, 'my link'). This is a consequence of it not being built-in.
  • We use router(router.page(), router.page(), ...)
  • router.page(path, [options], render), where:
    • path is a pattern, including :name which are placed into the params passed to options.state(params)
    • options (optional), an object containing the following (optional) functions:
      • state(params) a function that returns the new state given the params derived from the path. Can return a promise of new state. This is called when we go to a new URL. When we go back, or forward then this function is not called, the previous (cached) state at that URL is used.
      • url(state) a function that returns a URL based on the state or the rest of the model. If this is different to the current URL, we replace the URL and refresh the view. Replacing the URL means you cannot go back to the previous state... I think this is better than pushing a new URL. Perhaps an option here would be good?
      • binding the binding of the state onto the model. Set when the state(params) function returns new state. This state is written to the browser's state cache for the URL.
    • render a function returning the vdom for the given route.

In effect, we have the following features:

  • View is determined by the current pathname
  • When we navigate to a new route, the options.state(params) function is called for that route. This is written into the model using the options.binding.
  • State is stored in the history API.
  • When we go back, or forward to a route, we retrieve the state from the history API, options.state(params) is not called.
  • When the application changes the model's state (according to binding), the new state is stored in the history API, so when we go back to that route the modified state for that route is restored again.
  • The model can change the route, with the options.url(state) function. When the URL returned from options.url(state) is different from the current URL, we make a history API replace.

@refractalize refractalize merged commit d2a9dad into master Feb 6, 2015
@joshski
Copy link
Member

joshski commented Feb 6, 2015

woot

@joshski joshski deleted the routes branch February 20, 2015 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants