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

Docs: Code to listen for 'navigation' change does not output written behavior #627

Open
as-dr opened this issue Feb 13, 2018 · 5 comments
Open

Comments

@as-dr
Copy link

as-dr commented Feb 13, 2018

Expected behavior

In the Choo docs on Routing, in the section "Listening for Route Changes" implementing:

var html = require('choo/html')
var choo = require('choo')
var app = choo()

var app = choo()
app.use((state, emitter) => {            // 1.
  emitter.on('navigate', (route) => {    // 2.
    console.log(`Navigated to ${route}`) // 3.
  })
})

Should return a console message "Navigated to [insert route here]

Actual behavior

This code snippet instead returns a console message "Navigated to undefined"

Steps to reproduce behavior

  1. Add in code as written in the docs (and reproduced above)
  2. Click through / navigate between two separate routes.
  3. Observe the console message.
@marcbachmann
Copy link
Contributor

Here's the link to the routing documentation: https://github.com/choojs/website/blame/49f3b8d4aabbadaf67c13854fb3ce680a57374dd/content/docs/routing/index.md#L340

I don't think that this ever worked in v6... maybe in earlier versions.
The route is available on app.state.route.

@bates64
Copy link
Contributor

bates64 commented Feb 15, 2018

if we're considering changing the api here (to, from) would be cool.

@goto-bus-stop
Copy link
Member

yeah, knowing the previous route would be really useful!

@marcbachmann
Copy link
Contributor

marcbachmann commented Feb 15, 2018

We won't have access to the previous state anymore with the current setup.
Should we only pass specific properties?

... I'm also not sure how we should differentiate the state in before and after hooks.
This needs a proper proposal for all events/behaviors first. Here's also some comment regarding that: #613 (comment)

@kareniel
Copy link
Member

re: knowing the previous route:

How about adding a previousRoute property to the state?
We could copy it over when we _matchRoute:

 Choo.prototype._matchRoute = function (locationOverride) {
   var location, queryString
   if (locationOverride) {
     location = locationOverride.replace(/\?.+$/, '')
     queryString = locationOverride
   } else {
     location = this._createLocation()
     queryString = window.location.search
   }
   var matched = this.router.match(location)
   this.state.href = location
   this.state.query = nanoquery(queryString)
+  this.state.previousRoute = this.state.route
   this.state.route = matched.route
   this.state.params = matched.params
   this.state._handler = matched.cb
   return this.state

I'm looking at animating route transitions and having access to previous route (somehow, doesn't have to be this way) would be super useful.

As for the docs reflecting actual behavior I made a small pr here: choojs/website#61

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

No branches or pull requests

5 participants