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

Match initial route before stores are called #681

Closed
wants to merge 1 commit into from

Conversation

lachenmayer
Copy link

Hello choo croo, I just made a store that needs to get a URL param from state.params - I was only able to get it after the DOMContentLoaded event, which I thought was a bit weird.

Basically, when I navigate to /thingy/foo, I'd like to do this:

app.route('/thingy/:id', (state, emit) => html`<div>this is thingy ${state.params.id}</div>`)
app.use((state, emitter) => {
  console.log(state.params.id) // <= expected: foo
  // actual: state.params is undefined :(
  emitter.on('DOMContentLoaded', () => {
    console.log(state.params.id) // this works though
  })
})

I see no reason why I shouldn't be able to access the route info directly in the store - as far as I can tell, it doesn't interfere with any of the other things that happen on initial render.

This change just moves the this._matchRoute call in Choo.start & Choo.toString above the initial stores call.

I've added tests for state.params & state.href being set in the store, and also checked the expected behavior in my store :)

@goto-bus-stop
Copy link
Member

the ordering here is kinda tough but I think we want to be able to init stores before handling routes, so eg a store can add routes, allowing you to split things up into modules etc:

app.use(require('./routes'))
app.use(require('choo-login-page-module-from-npm'))

currently that doesn't work in bankai pending choojs/bankai#452 but imo, waiting for DOMContentLoaded would be the correct way to go.

does that make sense?

@lachenmayer
Copy link
Author

Hmm, that's an interesting usecase I hadn't considered! I wish I could at least get "best-effort" route info, but that could potentially be even more confusing... Will use DOMContentLoaded for now, cheers!

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.

2 participants