Skip to content

feat: add setAtom bound to the state manager #57

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

Closed
wants to merge 4 commits into from

Conversation

jodarove
Copy link

@jodarove jodarove commented Mar 27, 2025

Description

This PR proposes the addition of a new API for State Managers: setAtom, which allows state manager authors to directly update atom values without the need to use an update function. The API limits which atoms can be updated to only those that are created within the defineState function.

Before this PR:

const state = defineState((atom, _computed, update, _fromContext, setAtom) => () => {
  const count = atom(1);

  const increment = update({ count }, ({ count: countValue }) => ({
    count: countValue + 1,
  }));

  return {
    count,
    increment,
  };
});

After this PR:

const state = defineState((atom, _computed, _update, _fromContext, setAtom) => () => {
  const count = atom(1);

  const increment = () => setItem(count, count.value + 1);

  return {
    count,
    increment,
  };
});

Things to notice here:

  1. With this version need to access the .value because we are directly using the atom.
  2. Every operation after a setAtom, is affected by it because when setting it, it will mark all dependencies of this atom as “dirty” therefore recomputed in next access.
  3. the function can be async and return a value.
  4. ** Needs more thinking: what if the underlying implementation changes (ex, preact/signals), and increment is executed within, for example an effect, this this access will be register for that effect; therefore whenever count.value changes, the effect will be executed.

Followup proposal: can we remove the update function?

🟢 No breaking change

@@ -52,5 +52,6 @@ export type DefineState = <
computed: MakeComputed,
update: MakeUpdate,

Choose a reason for hiding this comment

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

can we do something to let consumers know that update is deprecated?

Copy link
Author

Choose a reason for hiding this comment

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

since the package version is 0.x.y i was thinking about removing the update functionality entirely (working on it in a different PR).

Copy link
Author

Choose a reason for hiding this comment

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

@@ -52,5 +52,6 @@ export type DefineState = <
computed: MakeComputed,
update: MakeUpdate,
fromContext: MakeContextHook<ContextShape>,
setAtom: <T>(a: Signal<T>, newValue: T) => void,

Choose a reason for hiding this comment

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

I'd like to rethink the atom/setAtom names at some point & see if we can come up with something that has more immediate / imperative connotations. Doesn't need to be in this PR, though.

Copy link
Author

Choose a reason for hiding this comment

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

sure thing, let's brainstorm on it!

@jodarove jodarove force-pushed the jodarove/update-proposal branch from f92226b to 56fa5fa Compare April 9, 2025 19:39
@jodarove jodarove closed this May 9, 2025
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