Skip to content

feat(StateStore): state modifiers and store merging #1461

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

Merged
merged 5 commits into from
May 29, 2025

Conversation

arnautov-anton
Copy link
Contributor

@arnautov-anton arnautov-anton commented Jan 31, 2025

Description of the changes, What, Why and How?

  • adds state preprocessors (like middleware for state change before state is commited to subscribers)
  • adds store merging

@arnautov-anton arnautov-anton force-pushed the feat/new-state-store-features branch from ff47a28 to 5f4f942 Compare January 31, 2025 16:31
@arnautov-anton arnautov-anton force-pushed the feat/new-state-store-features branch from 5f4f942 to 9995196 Compare May 11, 2025 14:02
Copy link
Contributor

github-actions bot commented May 11, 2025

Size Change: +5.08 kB (+1.24%)

Total Size: 414 kB

Filename Size Change
dist/cjs/index.browser.cjs 119 kB +1.68 kB (+1.43%)
dist/cjs/index.node.cjs 163 kB +1.71 kB (+1.06%)
dist/esm/index.js 132 kB +1.69 kB (+1.3%)

compressed-size-action

@arnautov-anton arnautov-anton force-pushed the feat/new-state-store-features branch 2 times, most recently from 7e34139 to 6d565a1 Compare May 11, 2025 14:56
@arnautov-anton arnautov-anton marked this pull request as ready for review May 11, 2025 15:06
@arnautov-anton arnautov-anton force-pushed the feat/new-state-store-features branch from 6d565a1 to 5408ab3 Compare May 12, 2025 07:48
originalValue !== this.cachedOriginalValue ||
mergedValue !== this.cachedMergedValue
) {
this.value = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to give a try to utils/mergeWith.ts? It could be handy for nested objects like message composer config is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will be needed as the mechanism does only a shallow merge due to potential type conflicts, see this part which dissalows merging two stores if they have top-level keys in common.

const storeA = new StateStore({ a: 1, b: 2 });
const storeB = new StateStore({ x: 1, b: 'aa' });

storeA.merge(storeB); // type error -> both stores have `b` state property

If this was not in place, the state would produce unexpected results whenever either of the states changed.

If storeA updates b to, let's say 10 - the merged store would look like this: { a: 1, x: 1, b: 10 } while the merged type would look like this: { a: number; x: number; b: string | number }. Subscribers with selector pointing to b would receive 10. Then storeB updates b to hello now subscribers with selector poiting to b would receive hello and suddenly there's no value/type stability - this does not seem that bad until you decide to get latest value and you need b value to be a number but since storeA hasn't changed since, you'll get hello instead.

Potential solution to this problem is to have the actual merged state object look like this:

type MergedStateStoreState<X, Y> = {
   original: StateStore<X>;
   merged: StateStore<Y>;
}

But even in this form there's no need for mergeWith just yet.

@arnautov-anton arnautov-anton force-pushed the feat/new-state-store-features branch from 5408ab3 to efb6d00 Compare May 29, 2025 08:49
@arnautov-anton arnautov-anton force-pushed the feat/new-state-store-features branch from 4ded36f to 1f5de53 Compare May 29, 2025 10:17
@arnautov-anton arnautov-anton merged commit 6887cd2 into master May 29, 2025
6 checks passed
@arnautov-anton arnautov-anton deleted the feat/new-state-store-features branch May 29, 2025 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants