-
Notifications
You must be signed in to change notification settings - Fork 597
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
Best pattern to fetch data based on a path #447
Comments
@qzapaia I'm wondering this too. Though I have some issue with this approach. The I have tried to emit the I have worked with React for a while, and normally we could simply use |
@qzapaia yeah I think that's quite reasonable |
Guys, I think this corresponds to my choo-routehandler framework. Bear in mind it's written for Choo v4, as I haven't got around to looking at v5 yet. However, the core premise of the framework is to facilitate the loading of data required by individual routes, if you see what I mean. So when a route is switched to, it may load data asynchronously if it defines a I also found |
@fraserxu ahh yeah so I think I figured out what's happening in your case: you're recreating the tag on each render, so the But actually I think using a one-off var might be just as easy: var html = require('choo/html')
var choo = require('choo')
var app = choo()
app.use(productStore)
app.route('/product/:id', mainView)
app.mount('body')
var loaded = false
function mainView (state, emit) {
if (!loaded) {
loaded = true
emitter.emit('fetchProduct');
}
return html`
<body>
<h1>
product is ${state.product.name}
</h1>
</body>
`
}
function productStore (state, emitter) {
emitter.on('fetchProduct', function () {
fetch('/product/'+state.params.id,function (product) {
state.product = product;
emitter.emit('render')
})
})
} |
Hi @yoshuawuyts thanks for the response. I think I've tried all the solutions here. The issue with using The issue with set a The issue to I've setup a demo repo to show what I mean and hopefully it makes more sense with real code. https://github.com/fraserxu/choo-data-fetching-demo |
@fraserxu oh but detecting route changes shouldn't be that hard - e.g. I think something like this should work: var html = require('choo/html')
var choo = require('choo')
var app = choo()
app.use(productStore)
app.route('/product/:id', mainView)
app.mount('body')
var route = null
function mainView (state, emit) {
var newRoute = window.location.href
if (newRoute != route) {
route = newRoute
emitter.emit('fetchProduct');
}
return html`
<body>
<h1>
product is ${state.product.name}
</h1>
</body>
`
}
function productStore (state, emitter) {
emitter.on('fetchProduct', function () {
fetch('/product/'+state.params.id,function (product) {
state.product = product;
emitter.emit('render')
})
})
} |
It does require keeping
The naming and routing store can probably be improved but that's what I came up with just now. If that's not ideal, someone let me know :O |
Just throwing my two cents in here :) Feel free to tell me if it's crazy, as either I'm doing it wrong or I may have found a bug with similar problems to #479/#549. Coming from react land, the event-emitter pattern seems very close to redux actions, so for loading data I would like to use a pattern like so: function todoStore(state, emitter) {
const todos = {todo: null}
state.todos = todos
emitter.on('navigate', ()=> {
if (someCondition) {
emitter.emit('todos.fetchTodo', state.params.id)
}
})
emitter.on('todos.fetchTodo', id => {
loadTodo(id).then(todo => emitter.emit('todos.fetchTodo.done', todo))
})
emitter.on('todos.fetchTodo.done', todo => {
todos.todo = todo
emitter.emit('render')
})
} By introducing a parameter for The issue I'm having is that the |
Is this resolved in a more elegant way? |
This is a very common issue ! |
@chuck911 Why ask for forgiveness when you're consciously rude? It's not that hard to give constructive feedback. "I see a lot of missing functionality in the routing system in Choo. It would be great if I were able to do X. I could try to create a pull request if someone would be willing to mentor/help me?" |
@chuck911 - You're being very hurtful by saying an open source library that has taken 100s of hours of people's free time sucks. What's more is you haven't suggested anything constructive that solves the issue at hand. Please take a moment before posting comments that purposefully upset people who are passionate contributors to open source. |
@josephluck @marlun Thank you very much, you are very kind. |
I've submitted a PR that might make this easier. Probably not quite ready for merge yet, but could be a starting point... |
@chuck911 - I respectfully disagree, there's already a few ways that the question in this issue can be addressed, and many people have been kind enough to share their experiences in this thread. Demanding the attention of the author in the way you have is pretty disrespectful. |
Let's keep this conversation positive. One of the reasons that choo is so great is that it keeps the core minimal by allowing others to come up with the userland solutions that work best for them. I just published |
Expected behavior
I was wondering if this is a good pattern in order to fetch some data when access some URL.
The text was updated successfully, but these errors were encountered: