keajs / kea

Batteries Included State Management for React
https://keajs.org/
MIT License
1.94k stars 51 forks source link

BindLogic + automatic connections support? #135

Closed cee-chen closed 2 years ago

cee-chen commented 3 years ago

πŸ‘‹ 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:

// engine_logic.ts
export const EngineLogic = kea<MakeLogicType<EngineValues, EngineActions>>({
  path: (key: string) => ['...', key],
  key: (props) => props.engineName,
  // ... etc
});

// bind logic setup
const { engineName } = useParams(); // derived from react router/browser URL

<BindLogic logic={EngineLogic} props={{ engineName }}
  <EngineRouter />
</BindLogic>

// usage
const { engineName } = EngineLogic.values;

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!

mariusandra commented 3 years ago

Hey, indeed BindLogic works just inside React. It uses a React Context to map any logic onto any props, and will read from this mapping inside the useValues and useActions hooks, and nowhere else.

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 multipleBindLogics at the same time on different parts of your app (e.g. three engines shown at the same time on one page). Suppose you have one UserLogic (no key) that's used somewhere inside all three <BindLogics>. If it has a listener that gets EngineLogic.values.something, then what should it have bound to? UserLogic is mounted just once without a key after all.

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 connect:

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 engineName from the props in each logic, and pass it to the other one.

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 EnglineLogic? Will there be multiple different engines on the screen at the same time? It may also be possible there's no automatic solution that can help... πŸ€”

cee-chen commented 3 years ago

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.

How central is EnglineLogic? Will there be multiple different engines on the screen at the same time?

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.

It may also be possible there's no automatic solution that can help... πŸ€”

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 EngineLogic.values.whatever from any given file/fn outweighs the extra overhead that comes from keyed logic πŸ˜… CCing for info @JasonStoltz and @byronhulcher - definitely feel free to disagree!

And as always, thanks for the amazing work/explanation Marius, I super appreciate your time!!

mariusandra commented 2 years ago

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. EnglineLogic.values.passengers, and considering the async callbacky nature of JS, it's not obvious how to do this automatically without a lot of magic boilerplate and wrappers like babel plugins.

I tried doing something similar to what's needed here. Kea 2.0 supported something called "auto-connect", where if you type EngineLogic.values.passengers in a listener, and EngineLogic is not mounted, it'll mount it automatically and connect to the current logic. To get this working, I had to create a call stack that tracks with listeners called which actions, and which listeners those actions triggered in return. This added a lot of weight to the library, and... well, usually worked.

Unfortunately, you can always await delay(10), and while we wait, some other code is doing its own thing, calling other actions, etc. When we return after the 10ms delay, the listener stack might be completely different... and we have no idea where in the execution trail we now are.

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 EngineLogic we must access. Just like with listeners, there's also no way to really know which logic we are running any code in, from the perspective of EngineLogic. Callbacks from other logics could mess up the stack.

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 ActiveEngineLogic, which would be keyless and contain the same actions and values, but always points to the right keyed logic.

Alternatively, just keep EngineLogic keyless and reload the entire page when it changes. That's what we did at PostHog. We have one teamLogic. Whenever we change the team/project, we reload the entire page. This way we can treat teamLogic as a global, and not use a selector or useValues for every URL that needs the team ID. Instead we can just call teamLogic.values.currentTeamId knowing that it's always there and never changes.

Hope this helps somewhat...

...

One other alternative that could work is the idea of connections. This is not real code, but such a plugin can be built. It'll bring a few new issues, but could work in the end. The basic idea is this:

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 connections and be stuck with the real EnglineLogic, as they have the same variable name... though it'll just throw then.

Would it be worth building such a plugin?

cee-chen commented 2 years ago

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,

Alternatively, just keep EngineLogic keyless and reload the entire page when it changes.

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.