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.

Sign up for GitHub

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

Jump to bottom

404 handled weirdly #34

Open
AdriVanHoudt opened this issue Nov 9, 2017 · 3 comments
Open

404 handled weirdly #34

AdriVanHoudt opened this issue Nov 9, 2017 · 3 comments

Comments

@AdriVanHoudt
Copy link

So this is probably a thing with the router or something but I found this through the app generated by this lib so I thought I would start here ^^

My route config looks like

app.route('/', require('./views/main'))
app.route('/title', require('./views/title'))
app.route('/test/:id', require('./views/test'))
app.route('*', require('./views/404'))

after adding 1 test route (with nothing special in it, just prints out the state)

Now when I add a button that does a pushState to /test/4?a=4 that works great.
Then I reload the current page and I get No route found for /test/1?a=4.

I tried some stuff and saw that choo passes a default /404 route for, well, 404's :P which isn't there.
I would suspect it to go to * maybe but that doesn't seem the case.

Now when I add

app.route('/404', require('./views/404'))

Everything seems to work, reloading on the state actually loads the right view.

I'll be glad to look into this more or give you more info if you need it
@yoshuawuyts
Copy link
Member

Hmmm, I'm guessing this is a failure of Server Side Rendering; probably missing to pick up a route in Bankai. What's your version of Bankai, Node, and NPM? Thanks a lot for raising this!

cc/ @Flet didn't we publish a patch for this recently? I feel we there might perhaps still be a bug somewhere with the Server Render logic.

@AdriVanHoudt
Copy link
Author

Bankai: 9.0.0-rc9
Node: 8.9.0
npm: 5.5.1

Didn't even know it was doing ssr /mindblown :D

@Flet
Copy link

Flet commented Nov 13, 2017

Hello!

I just tried this out with create-choo-app (which is currently bankai@9.0.0-rc10) and it seems to be working ok?

I added this route to index.js: (Just above the 404 route)

app.route('/test/:id', require('./views/test'))

And I added this file
views/test.js:

var html = require('choo/html')
module.exports = view

function view (state, emit) {
  return html`
    <body class="sans-serif">
      <p>
        Foo is ${state.params.id}
      </p><p>
        Bar is ${state.query.bar}
      </p>
    </body>
  `
}

Then npm start and navigate to https://localhost:8080/test/123?bar=1 and it works!

@AdriVanHoudt Could you update to rc10 and give it another try?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Flet @AdriVanHoudt @yoshuawuyts