Easy-peasy: RFC: computed API (DEPRECATES selector)

Created on 20 Jun 2019  Β·  37Comments  Β·  Source: ctrlplusb/easy-peasy

Updated with Proposal Number 2, see below


Hello all πŸ‘‹

First of all, I would like to apologise for the churn on the selectors API, with the deprecation of select, being replaced with the selector helper. Overall the v2.4.0 changes have been well received, especially the new listeners API.

That being said I have definitely noted that the new selector API seems to be causing some friction. I would like to address this by opening an RFC process with the hope to get some feedback on some ideas I have to make the API more palatable. Once again I apologise for the churn on this - especially on the early adopters whom have been refactoring their applications.

Personally I am very happy with the rest of the Easy Peasy API, but agree that the selector API isn't in line with the experience offered by the rest of the API. I want to address this as I feel once this is resolved I am happy to freeze the API, cut v3 (which would remove all the deprecated code and allow me to fully realise some nice performance benefits) and then focus on long term support.

With this in mind I would like to put forward my first proposal for the refactored selector API. I will be building this out on a separate branch which will be published under the v3 alpha tag.

I invite you to provide criticism to the proposal, and/or create proposals of your own. It would be great to get a wide consensus on the API. πŸ‘


Proposal 1

This proposal has been removed as there is a clear rally around Proposal 2


Proposal 2

Ok, some really great feedback from @jmyrland and @macrozone has made me push into another proposal. This proposal is somewhere between select and selector. I've move back to computed properties, as in the select, but I am using getter logic under the hood to keep it performant, only doing the computation when accessed.

In addition to this I have included aspects from the selector API, namely the ability to define state resolvers which allow you to isolate the state that your computed property depends on whilst also granting you the ability to access the entire store state.

Overall I am feeling really good about this proposal, but once again I invite all criticism. Please don't hold back. :)

I've also moved away from the previous naming. The name for this helper will be computed - much more inline with the fact that it is now a computed property.

API

computed(computationFunc, stateSelectors)

Binds a computed property against your model. It takes state and returns new derived state, which is accessible as a normal property.

Computed properties only have their values calculated when the property is accessed, i.e. lazy calculation. This allows for good performance characteristics. In addition to this the results of computed properties are cached, and will only be recomputed if the input state has changed.

isLogged: computed(state => state.user != null)

Arguments

  • computationFunc (Function, required)

The computationFunc will receive the input state and returns the derived value.

  • stateSelectors (Array, optional)

    Allows you to isolate the specific parts of your state that your computed property depends on, whilst also allowing you to resolve state from the entire store to use within the computationFunc.

    Using stateSelectors allows you to increase the performance characteristics of your computed properties as they will only need to be recomputed when the state resolved by the stateSelectors changes.

    Each stateSelector receives the following arguments:

    • state (Object)

    The local state against which your computed property is bound.

    • storeState (Object)

    The entire store state

    It is worth noting that the state and storeState that are provided to your state selectors will include computed properties too. Computed properties are allowed to reference each other.

    I would also recommend that you don't use stateSelectors unless you anticipate or experience potential performance issues. The simplicity of a computed property without state selectors makes it much easier for others to read and understand your code.


Defining computed properties

This section will demonstrate various use cases in terms of defining computed properties.

Simple computed property acting against local state

const model = {
  session: {
    user: { username: 'bob' },
    isLoggedIn: computed(state => state.user != null)
  }
}

A computed property isolation local state via a state resolver

const model = {
  products: {
    items: [{ name: 'boots', price: 20 }],
    totalPrice: computed(
      items => items.reduce((total, product) => total + product.price, 0)
      [
        state => state.items
      ]
    )
  }
}

A computed property isolation multiple parts of local state via a state resolver

const model = {
  todos: {
    items: {
      1: { id: 1, text: 'learn easy peasy' }
    },
    idsOfCompleted: [1],
    completedTodos: computed(
      (items, idsOfCompleted) => idsOfCompleted.map((id) => items[id]),
      [
        state => state.items,
        state => state.idsOfCompleted
      ]
    )
  }
}

A computed property using state resolvers to resolve local and store state

const model = {
  products: {
    items: {
      1: { id: 1, name: 'boots', price: 20 }
    },
  },
  basket: {
    productIds: [1],
    productsInBasket: computed(
      (productIds, products) => productIds.map(id => products[id]),
      [
        state => state.productIds,
        (state, storeState) => storeState.products.items
      ]
    )
  }
}

Accessing other computed properties within a computed property

const model = {
  products: {
    items: {
      1: { id: 1, name: 'boots', price: 20, stock: 3 }
    }
  },
  productList: computed(state => Object.values(state.products)),
  productsWithLowStock: computed(
    state => state.productList.filter(product => product.stock < 5)
  )
}

Accessing computed properties from store state

const model = {
  products: {
    items: {
      1: { id: 1, name: 'boots', price: 20, stock: 3 }
    },
    productList: computed(state => Object.values(state.products))
  },
  special: {
    discount: 0.5,
    productsWithDiscountPrice: computed(
      (discount, products) => products.map(product => ({
        ...product,
        price: product.price - (product.price * discount)
      }),
      [
        state => state.discount,
        (state, storeState) => storeState.products.productList,
      ]
    )
  }
}

Accessing computed properties

Now we will cover the various ways you can access computed properties.

Accessing via a component

import { useStoreState } from 'easy-peasy';

function TotalPriceOfProducts() {
  const totalPrice = useStoreState(state => state.products.totalPrice);
  return <div>Total: {totalPrice}</div>
}

Note how you access it just like any other state, exactly how it was with the deprecated select API.

Accessing via the store instance

console.log(store.getState().products.totalPrice);

Final Notes

I feel like this proposal feels much stronger than the first proposal as it reduces the complexity a fair deal, bringing us fairly close to the awesome dev experience that the select API provided.

Ok, all good I hope? One slight downer. immer doesn't support getter properties, which is exactly what we need to support computed properties. I forked immer and released a immer-peasy, which instead of throwing errors for getter properties, simply ignores them. It was a fairly small change to their code, but this means I will manually have to rebase and publish this package should this proposal gain the backing of the community.

Currently available via:

npm install [email protected]

Most helpful comment

Amaze, thanks for that @jmyrland.

I've been updating the website, so once we are covered there I may push for a release ASAP.

Really stoked to hear the migration process is straight forward.

All 37 comments

@macrozone @jmyrland @dai4 @homura @ntt2k

Well written and formulated πŸ‘ First things that comes to mind:

  • I got a little confused regarding the selecor-part, of injecting dependencies first - then defining the resulting function. Personally, I like the previous change of defining dependencies separately (sort of like the hooks API).

  • Accessing selectors: "No more function calls." - this is a great one! πŸ‘

  • In regards to the useStoreSelector - I'm not sure what implications this has. This may "complicate" things for users of easy-peasy wanting to get both state & selectors. When I think of (using) selectors, I think them as "computed properties part of the state"

@jmyrland thanks for the feedback.

I got a little confused regarding the selecor-part, of injecting dependencies first - then defining the resulting function. Personally, I like the previous change of defining dependencies separately (sort of like the hooks API).

Do you mean you prefer an API like so:

const model = {
  todos: {
    items: {
      1: { id: 1, text: 'learn easy peasy' }
    },
    idsOfCompleted: [1],
    completedTodos: selector(
      (items, idsOfCompleted) => idsOfCompleted.map((id) => items[id]),
      [
        state => state.items,
        state => state.idsOfCompleted,
      ]
    )
  }
}

I'd be interested in hearing what others think of this. I do like that the most important part is at the top.

In regards to the useStoreSelector - I'm not sure what implications this has. This may "complicate" things for users of easy-peasy wanting to get both state & selectors. When I think of (using) selectors, I think them as "computed properties part of the state"

Yeah, I agree that this may introduce some more complexity into the system, but there really is a distinction between state and selectors under the hood. I worry that without maintaining this distinction via the consuming APIs we may fall into other pitfalls.

For e.g. @macrozone had initially recommended we access the selector directly via the useStoreState selector, with the suggestion that the selector be automatically executed under the hood.

function CompletedTodos() {
  const todos = useStoreState(state => state.todos.completedTodos);
}

This works out in the basic case, but presents issues when a user resolves state with a nested selector.

e.g.

function App() {
  const todos = useStoreState(state => state.todos);

  // todos.completedTodos is a selector! Eek! 
}

So even if we do mount selectors on state the user is still forced to know which piece is native state vs a selector, and they would have to execute selectors in order to get the value.

The only way I can think out of this is using a Proxy / getters, but not sure how practical that is, especially with an underlying Redux store.

the old select api wrote the computed state back into the state object. I found that quite convenient, altough its redundant. So useStoreState would not need to distinguish between computed state and raw state.

Is passing props to the selector really that important? So far i did not stumble upon a reason to do so.

maybe we should reintroduce select as some derivedState helper?

having said that: i like the approach of using useStoreSelector, because as @ctrlplusb correctly pointed out, it is not the same thing.

It's then similar to how in classic redux, you use either connect with normal stateToProps-functions or introduce reselect. You have to do this distinction.

On the other side I also see the benefits of having a duplicated, computed state like written in the previous comment like we had with select

I am not sure what the best solution is, maybe both.

Do you mean you prefer an API like so:

@ctrlplusb yeah - personally I prefer that convention. But I agree witn @macrozone - is there really a need for selector parameters? I assume it has something to do with tracking changes?

This is the most intuitive for me:

selector(state => state.idsOfCompleted.map(id => state.items[id])),

maybe we should reintroduce select as some derivedState helper?

This sounds like a good idea - this will enable easier migration to the new version , as you can limit changes to stores πŸ‘

Yeah, I agree that this may introduce some more complexity into the system, but there really is a distinction between state and selectors under the hood. I worry that without maintaining this distinction via the consuming APIs we may fall into other pitfalls.

In that regard, I see your point. But for consumers, I don't think splitting selectors into a different hook is of any real value.

When I think of selectors - I think of computed properties in Vue. "Consumation" of regular data and computed properties are the same - but the underlying functionality is different.

having said that: i like the approach of using useStoreSelector, because as @ctrlplusb correctly pointed out, it is not the same thing.

I get that it is different internally - but my assumption is that the consumer don't really care if it is a selector or a state property. If people care of differentiating between selectors and state properties - I actually prefer the current version of exposing functions from useStoreState.

The only way I can think out of this is using a Proxy / getters, but not sure how practical that is, especially with an underlying Redux store.

This was my initial thought as well. I really think abstracting selectors behind a getter would simplify selector usage. However, it may - as you state - introduce additional complexity under the hood.

I don't really know the internals of easy-peasy / redux well enough to say if this approach would work - so I trust your judgement πŸ‘

Thanks both, lots of food for thought. I have something interesting brewing in my mind. I will compose a reply to you both later along with Proposal 2.

Been great feedback, this has really made a big impact on my thoughts around this API. πŸ‘

@jmyrland @macrozone

I took the feedback from you both and have created Proposal 2. Check the OP.

Actually, thinking about this, in the context of computed we could allow a shorthand version. If you don't provide the stateResolver it will default to passing the state into your resolver. In most cases this is probably fine.

So,

const model = {
  products: {
    items: [{ name: 'boots', price: 20 }],
    totalPrice: computed(
      items => items.reduce((total, product) => total + product.price, 0)
      [
        state => state.items
      ]
    )
  }
}

Could be replaced with

const model = {
  products: {
    items: [{ name: 'boots', price: 20 }],
    totalPrice: computed(state => 
      state.items.reduce((total, product) => total + product.price, 0)
    )
  }
}

Thoughts? If you like this I will amend the proposal.

Screw this I love the shorthand verions, I am going to update the proposal!

Proposal 2 looks really, really strong, Sean! Some things I love:

  • It returns to the simplicity of the deprecated select API. That felt the most natural to meβ€”just write a function that accepts state as an argument and returns a computed value.
  • It allows accessing computed values as property names (like before, i.e. state.sliceOfState.computedValueName) rather than calling them as functions
  • I love that you can _optionally_ include stateSelectorsβ€”again, this really simplifies the API, but still allows for more complex use cases.

Thanks for the awesome work on this. If you decide to release 2.6 with Proposal 2 I'd love to test it out and see if the bugs I faced with the selector API in 2.4 are resolved.

Awesome to hear @nhuesmann - really happy you are willing to put yourself forward for trying out the alphas. I know it must be a pain to refactor your code so often. Hopefully after this we will have a much more stable API with a focus on long term support.

@ctrlplusb It's no trouble at all, manβ€”I don't mind the refactors, and I'm still in an early alpha version of my project so it doesn't negatively impact me. I'm just excited about the direction this lib is headed so I'm committed to keeping up with it. Bring on the alphas!

@nhuesmann and everyone else whom is willing. I have published the Proposal 2 under the next tag.

npm install easy-peasy@next

Early adopters that provide feedback will be paid in heart emoji. 😘

@ctrlplusb I will upgrade as soon as I hop into work again and will let you know how it goes, pumped to try it!

This is looking real good @ctrlplusb! πŸ‘

I'll try out the changes in our project and let you know how it works out πŸ‘

Heart emojis as promised

Tested easy-peasy@next (actual 2.6.0-alpha.0) for a project running [email protected] with [email protected].

Nothing breaks on the initial run πŸ‘ (we have not yet adopted selector, but we have a bunch of selects)

I tried converting one of the selects - and it was as easy as just replacing select with computed 😁 After the change - everything works as expected! πŸ‘ Also, the computed prop is no longer on the "state object" (when inspecting in redux dev tools) πŸ‘

(Sidenote: I had an issue with my IDE (vscode) not registering the new computed types. When importing the new types, I got an error (no such type in easy-peasy). To resolve this, I just opened the index.d.ts file of easy-peasy, and the error was resolved πŸ‘)

I have some other intricate cases with selects depending on other selects and selects used within actions that I'm gonna convert to computed - but I've tested that a simple use case for computed works! πŸ‘

Found an issue in one of my intricate models, with selects within actions.

I made a simple test which contains the issue after replacing select with computed.

import { Action, action, Computed, computed, createStore } from 'easy-peasy';

interface ITestModel {
  numbers: number[];
  squaredNumbers: Computed<ITestModel, number[]>;
  setNumbers: Action<ITestModel, number[]>;
}

const model: ITestModel = {
  numbers: [],
  squaredNumbers: computed(state => state.numbers.map(n => n * n)),
  setNumbers: action((state, payload) => {
    //                            πŸ‘‡ is undefined - expected []
    const existingSquares = state.squaredNumbers;
    // Remove existing "squares" from the payload
    state.numbers = payload.filter(
      //         πŸ‘‡ is undefined - πŸ’₯
      number => !existingSquares.includes(number)
    );
  }),
};

test('Accessing computed props within actions should not fail', () => {
  const store = createStore(model);
  store.dispatch.setNumbers([1, 2, 3]);
});

I'm having an issue with the TypeScript definitions. Here's my setup, with visual aids via emojis in the code as @jmyrland so cleverly did above πŸ‘†.

type ActiveAges = [boolean, boolean, boolean];

interface AgesModel {
  activeAges: ActiveAges;
  isClearAgesButtonHidden: Computed<AgesModel, boolean>;
}

const ages: AgesModel = {
  activeAges: [true, true, true],
                                  // πŸ‘‡ISSUE A: state implicitly has an 'any' type
  isClearAgesButtonHidden: computed(state =>
    isEqual(state.activeAges, defaultActiveAges)
  ),
  ...
}

ISSUE A:
The state argument given to the computationFunc does not pick up my store model interface definitionβ€”it has an implicit 'any' type.

I tried adding a stateSelector to see what happened. Here is my code and the result (the AgesModel from before is unchanged):

const ages: AgesModel = {
  activeAges: [true, true, true],
  isClearAgesButtonHidden: computed(
   // πŸ‘‡ ISSUE B: now has a weird type that I'll add in notes below
    activeAges => isEqual(activeAges, defaultActiveAges),
   // πŸ‘‡ ISSUE C: TypeScript is unhappy, I'll detail below
    [state => state.activeAges]
  ),
  ...
}

ISSUE B:
The slice of state I've isolated using the stateSelector (activeAges) gets typed as:

(parameter) activeAges: Compact<{} & Compact<{
    ages: [AgeInterval, AgeInterval, AgeInterval];
    activeAges: [boolean, boolean, boolean];
    isClearAgesButtonHidden: boolean;
    agesQueryCriteria: string;
} & {}>>

Is this correct? It looks like an object containing the entire state, rather than just the field I've plucked off state using the stateSelector. I'd expect it to have the type ActiveAges that I defined in the file.

ISSUE C:
TypeScript is yelling about the stateSelector function typing. Here's what it says:
Argument of type '((state: any) => any)[]' is not assignable to parameter of type 'void'.

That's what I've got so far. I'll see if I can somehow silence the TypeScript errors and actually test the code functionality, but I use TypeScript exclusively in this project so I'd need to clear up these warnings so VS Code stops highlighting the files red so I can sleep at night. Will report back if I get to test the functionality today!

Hey both, awesome work here. πŸ‘

@jmyrland - good catch. I added the test case and have a fix applied. This will work in the next alpha release. πŸ‘

@nhuesmann - I need more time to investigate your case. In the interim would you mind letting me know what version of Typescript you are using?

@jmyrland - fix released in [email protected] πŸ‘

@ctrlplusb I'm on TypeScript 3.3.3 β€” 3.5 just breaks VS Code for me :(. You think that could be part of it?

@ctrlplusb UPDATE: I tested using VS Code's version of TypeScript (3.5.1). Issue A is resolved, however B and C still apply. If I just use a computationFunc it's fine, however when I add a stateSelector issues B and C occur still. Hope this helps!

@nhuesmann thanks for this. I have made some big improvements to the Typescript definitions. I tested on 3.3 and 3.4 and 3.5 - all good. Just need to finish off some cleanup and write up some docs in regard to them. Hopefully I will have time later this eve. πŸ‘

@ctrlplusb I love proposal 2, could not try it out yet, but will give it a shot asap! πŸ‘

@ctrlplusb I love proposal 2, could not try it out yet, but will give it a shot asap! πŸ‘

@nhuesmann I have fixed the types, tested on TS >= 3.3

npm install [email protected]

I haven't had time to write the docs for the types, but here is an example from the tests I wrote for the types.

import { Computed, computed, ResolvedState1, ResolvedState2 } from 'easy-peasy';

interface Product {
  id: number;
  name: string;
  price: number;
}

interface ProductsModel {
  products: Product[];
  // πŸ‘‡ A simple computed prop, with no resolved state
  totalPrice: Computed<ProductsModel, number>;
  // πŸ‘‡ A computed prop that resolves 1 piece of state via state resolvers
  totalPriceVerbose: Computed<ProductsModel, number, ResolvedState1<Product[]>>;
}

interface BasketModel {
  productIds: number[];
  //     A computed prop that resolves 2 pieces of state via state resolvers
  // πŸ‘‡ whilst also resolving global state
  products: Computed<
    BasketModel,
    Product[],
    ResolvedState2<number[], Product[]>,
    StoreModel  // πŸ‘ˆ important to provide store model when resolving global state
  >;
}

interface StoreModel {
  products: ProductsModel;
  baskets: BasketModel;
}

const mode: StoreModel = {
  products: {
    products: [{ id: 1, name: 'boots', price: 20 }],
    totalPrice: computed(state =>
      state.products.reduce((total, product) => total + product.price, 0),
    ),
    totalPriceVerbose: computed(
      products => products.reduce((total, product) => total + product.price, 0),
      [state => state.products],
    ),
  },
  baskets: {
    productIds: [1],
    products: computed(
      (productIds, products) =>
        productIds.reduce<Product[]>((acc, id) => {
          const product = products.find(p => p.id === id);
          if (product) {
            acc.push(product);
          }
          return acc;
        }, []),
      [
        state => state.productIds,
        (state, storeState) => storeState.products.products,
      ],
    ),
  },
};

I export the following types to represent the various numbers of state being resolved:

import { 
  ResolvedState1, 
  ResolvedState2, 
  ResolvedState3, 
  ResolvedState4, 
  ResolvedState5 
} from 'easy-peasy';

If in practice people are resolving more than 5 pieces of state we can extend the types.

You can alternative use the "tuple" form, but using these aliased types makes it a bit easier to read your model definitions. Hope that makes sense.

Shout if this solves your issues. πŸ‘

@jmyrland - you may be interested in this too as you are using TS too.

Looks good @ctrlplusb πŸ‘

Is there any difference in performance of totalPrice vs totalPriceVerbose? If so, is it significant?

    totalPrice: computed(state =>
      state.products.reduce((total, product) => total + product.price, 0),
    ),
    totalPriceVerbose: computed(
      products => products.reduce((total, product) => total + product.price, 0),
      [state => state.products],
    ),

@ctrlplusb awesome, thanks for the TypeScript fixes- I should be able to test it out today or tomorrow, I'll let you know how it goes!

@jmyrland - I would say there is no significant difference, only in the absolute exceptional cases (similar to what I describe here). The primary performance benefit comes from us making them lazy. πŸ‘
In fact we should take care in the docs to make sure that the simpler form is used over the state resolver form unless you anticipate perf issues (e.g. other parts of local state get updated extremely frequently).

@nhuesmann - nice one πŸ‘

@ctrlplusb I tested out the latest changes in 2.6.0-alpha.2, everything works perfectly! Upgrade was painlessβ€”just swapped out the Select and select keywords with Computed and computed keywords and everything just worked. Thanks for getting this to work!

@nhuesmann - that is music to my ears. πŸ™Œ

@macrozone @jmyrland - I'm interested in hearing your experiences with the latest.

If all are happy I think we should cut this fairly soon - I'd like to try to minimise the pain for people trying to upgrade to the current selector API, only to then have to upgrade to computed. The select -> computed upgrade is far easier.

Working with alpha.2, I've gone through all models - updated to computed. It took less than 2 minutes, utilizing "f2"-renaming in VSCode πŸ‘ (and we had a lot of references to select)

All tests pass - so I assume everything is working as expected. I had some snapshot tests of state objects, which confirms that computed properties no longer exist on the "state object".

I won't be going over "optimizing" computed properties, by defining "state selectors" - but I will check it out if we experience any "issues" in regards to performance. For now, I cannot see any issues πŸ˜„

TLDR; Good stuff! πŸ‘

Amaze, thanks for that @jmyrland.

I've been updating the website, so once we are covered there I may push for a release ASAP.

Really stoked to hear the migration process is straight forward.

works for me so far! but I have only some simple use cases for computed so far that work flawlessly

Was this page helpful?
0 / 5 - 0 ratings