-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
BindLogic + automatic connections support? #135
Comments
Hey, indeed The biggest blocker for making Kea work the way you described above, is that the React component layer and the Kea logic layer have overlapping, yet separate and parallel hierarchies. For example, you can use multiple These kinds of loose ends make it hard to support this. There might be alternative ways to make this work. The way you can currently pass props inside a logic is via const otherLogic = kea({
key: props => props.engineName,
connect: (props) => ({
values: [EnglineLogic({ engineName: props.engineName }), ['passengers', 'ticketsSold']]
}),
actions: {
doSomething: true,
}
listeners: ({ values }) => ({
doSomething: () => {
console.log(values.passengers)
}
})
}) ... but that requires quite a lot of manual plumbing. I could definitely simplify it down to something like this: const otherLogic = kea({
key: props => props.engineName,
connect: (props) => ({
// this does not work yet
logic: [EnglineLogic({ engineName: props.engineName })]
}),
actions: {
doSomething: true,
}
listeners: ({ values }) => ({
doSomething: () => {
console.log(EnglineLogic.values.passengers)
}
})
}) ... but you'd still have to grab the I don't know enough of your app to know if this would be a viable solution or just kicking the can a little bit further down the road. Would it be enough? Any other ideas on what could help? How central is |
Thank you for the extra context Marius!! Apologies for my ignorance, that does indeed all sound super complex 🙈 Unfortunately we also use automatic connections outside of other Kea logic files as well (e.g., in plain React components, or occasionally in plain JS util fns). For that use case, it definitely sounds like we couldn't key EngineLogic.
Super central to the app, there should only ever be 1 main EngineLogic functioning on the screen at a time, although EngineLogic does have to be reset when navigating between engine URLs. We do some extra cleanup/checks between engines based on React Router params to ensure that's the case and were hoping to simplify that code slightly with keyed logic/BindLogic.
Agreed there may be no easy/automatic solution! To be honest and just my 2c, but I sorta think the dev convenience of just being able to access And as always, thanks for the amazing work/explanation Marius, I super appreciate your time!! |
Hey @constancecchen, I'm coming back to this issue after a long hiatus :). The conclusion unfortunately is that there's no easy way to support this. You'd need to keep track of where it is that you're calling e.g. I tried doing something similar to what's needed here. Kea 2.0 supported something called "auto-connect", where if you type Unfortunately, you can always Basically: listeners(({ actions }) => ({
doSomething: async (_, breakpoint) => {
// last action called was `doSomething`
delay(5).then(() => {
actions.callBlaWithListener()
// Is the last action still `doSomething`?
})
await delay(10)
console.log('where are we?')
// Is the last action still `doSomething`?
// How should EngineLogic know that it's here? It's not that obvious :)
},
callBlaWithListener: async (_, breakpoint) => {
// last action called was callBlaWithListener
await delay(1000)
}
})) Very similar tracking code is needed to know which There are ways to work around many of the issues, but they're all workarounds which make everything more complex and slower. So in the end I decided to just cut the entire thing to simplify the stack. ... For your case, is it even an issue anymore? One alternative approach could be to create something like Alternatively, just keep EngineLogic keyless and reload the entire page when it changes. That's what we did at PostHog. We have one Hope this helps somewhat... ... One other alternative that could work is the idea of import { kea } from 'kea'
import { connections } from 'kea-connections' // does not exist
import { EngineLogic } from './EngineLogic'
kea([
connections((props) => ({
EngineLogic: EngineLogic({ engineName: 'bob' }),
})),
listeners(({ actions, connections: { EngineLogic }}) => {
console.log(EngineLogic.values.engineName) // taken from connections
})
]) It's not automatic... but could this improve your project? Do you see any cases where this could backfire? The biggest issues I can see is that it's super easy to forget to extract Would it be worth building such a plugin? |
Hey hey Marius, thanks as always for your time! ❤️ I ended up switching to a different team that no longer uses Kea, but I believe @JasonStoltz and @byronhulcher are still on it and can maybe answer your question as to whether it would be worth it. For what it's worth,
I believe this is what we were doing! So there's definitively all the alternatives you listed, and the combination of BindLogic + automatic connections is at most a syntactical nice to have. |
👋 Hey Marius!! It's us again, hope you're doing well :)
We're using BindLogic for our keyed logic files and it's working super great for us. There's one use case that it doesn't appear to work for, which is when we're using automatic connections both inside and out of other Kea logic files. Here's our example logic that we're attempting to key:
Engines are the powerhouse of our app so we do a bunch of referencing to
EngineLogic.values.xyz
all over the app. Unfortunately it looks like those aren't inheriting the keyed/bound logic and are causing console errors:Am I missing something, or is BindLogic + automatic connections not yet fully supported? I'm not sure how difficult it would be to add (I imagine very 😅), but we'd absolutely love that functionality over here if/when you have time! Thanks again for all your great work!
The text was updated successfully, but these errors were encountered: