keajs / kea

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

Prop values in selectors - retrieving via an inline selector doesn't seem to work correctly #124

Closed JasonStoltz closed 3 years ago

JasonStoltz commented 3 years ago

We are attempting to use a Prop inside of a Selector.

We're following this suggestion: https://kea.js.org/docs/guide/advanced#props-in-selectors

However, for whatever reason, we cannot seem to get access to props inside the inline selector as described in the documentation.

Here is a quick snippet I built on https://npm.runkit.com/kea to illustrate the issue.

require("react/package.json"); // react is a peer dependency. 

require ("react-dom");

require("react-redux/package.json"); // react-redux is a peer dependency. 

require("redux/package.json"); // redux is a peer dependency. 

require("reselect/package.json"); // reselect is a peer dependency. 
var kea = require("kea")

const logic = kea.kea({ 
  selectors: ({ props }) => ({
        valueFromSelectorsProps: [
            () => [],
            () => props.foo
        ],
        valueFromInlineSelectorProps: [
            () => [
                // inlineProps is an empty object here, it should be the props which I passed into the 'build' method
                (_, inlineProps) => inlineProps.foo
            ],
            (foo) => foo
        ],
  })  
})

logic.build({ foo: 'foo' })
logic.mount();

console.log(logic.values.valueFromSelectorsProps);
// "foo" --- It works!

console.log(logic.values.valueFromInlineSelectorProps);
// undefined --- It doesn't seem to work!

As you can see, I attempt to use the 2 different methods of accessing props as described in the docs. Both should work, right? For whatever reason though, I cannot get inline selector method to work.

Anyone have a second to take a look at this? Could just be user error.

JasonStoltz commented 3 years ago

Ohhhh, you know what. I think I solved this issue. Looks like to get it to work I actually need to save a reference to the result of build.

const instance = logic.build({ foo: 'foo' })
instance.mount();

console.log(instance.values.valueFromSelectorsProps);
//"foo" --- It works!

console.log(instance.values.valueFromInlineSelectorProps);
//"foo" --- It works!
mariusandra commented 3 years ago

Hey, that's indeed the right approach. If you call logic.mount(), it'll basically run logic.build({}).mount() behind the scenes, thus resetting the props you gave with logic.build({ foo: 'foo' }) just one line earlier.

JasonStoltz commented 3 years ago

I think the pain point for us here... is it appears that I now have to save a reference to the built logic to make this work. Is that correct? I cannot access these selectors via the wrapper?

// wrapper
logic.build({ foo: 'foo' }).mount();
console.log(logic.values.valueFromInlineSelectorProps);

vs.

// built logic
const instance = logic.build({ foo: 'foo' })
instance.mount();
console.log(instance.values.valueFromInlineSelectorProps);

It's a pain point just because we usually have the logic wrapper available globally, so it now requires us to pass around a reference. This is mostly a pain in tests.

I think it's just really confusing that props are apparently available in selectors, via the wrapper, if you're accessing them with selectors: ({props}) =>... but not when accessing them via an inline selector (_, inlineProps) => inlineProps.foo.

I guess I'm wondering if that is expected, or if it's potentially a bug, as it doesn't seem to be a documented behavior.

Thanks for your help!

JasonStoltz commented 3 years ago

Also just wanted to give you all a huge thank you for this project. We really love it!

mariusandra commented 3 years ago

Fixed in 2.3.5 :)

Turns out .mount() was basically calling .build().mount() and the default props in .build(props = {}).mount() were overriding the ones stored on the built logic. Since no other test broke, I just released the fix in a patch version.

The const instance = logic.build(props) approach makes sense if you're using keyed logic. Then each instance will have a different set of props attached to it.

And thanks for all the warm feedback! 😊