Skip to content
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

RESET symbol should be exported from jotai/utils #217

Closed
JLarky opened this issue Nov 29, 2020 · 8 comments · Fixed by #221
Closed

RESET symbol should be exported from jotai/utils #217

JLarky opened this issue Nov 29, 2020 · 8 comments · Fixed by #221

Comments

@JLarky
Copy link
Contributor

JLarky commented Nov 29, 2020

Here's the code that I want to write:

import { atom } from "jotai";
import { atomWithReset, RESET } from "jotai/utils";

export const tempFahrenheitAtom = atomWithReset(32);
export const tempCelciusAtom = atom(
  (get) => ((get(tempFahrenheitAtom) - 32) * 5) / 9,
  (get, set, newValue: number | typeof RESET) =>
    set(
      tempFahrenheitAtom,
      newValue === RESET ? newValue : (newValue * 9) / 5 + 32
    )
);

// in component
const reset = useResetAtom(tempCelciusAtom);

I created a sandbox that has an example of the code and possible workarounds https://codesandbox.io/s/jotai-atom-with-reset-bug-k6xcu?file=/src/atoms2.ts as well as recoil analog

import { atom, DefaultValue, selector } from "recoil";

export const tempFahrenheit = atom({
  key: "tempFahrenheit",
  default: 32
});

export const tempCelcius = selector<number>({
  key: "tempCelcius",
  get: ({ get }) => ((get(tempFahrenheit) - 32) * 5) / 9,
  set: ({ set }, newValue) =>
    set(
      tempFahrenheit,
      newValue instanceof DefaultValue ? newValue : (newValue * 9) / 5 + 32
    )
});

// in component
const reset = useResetRecoilState(tempCelcius);
@dai-shi
Copy link
Member

dai-shi commented Nov 29, 2020

Hmmmm, I see your use case... but, I hesitate exporting the symbol. Would like to hide it as an implementation detail.
Your example is totally valid and compelling. Hadn't thought about it.
Could we just export types? export type ...
Any other suggestions?

@JLarky
Copy link
Contributor Author

JLarky commented Nov 29, 2020

Yes, exporting just the type is what this code does:

import { atomWithReset } from "jotai/utils";

export const tempFahrenheitAtom = atomWithReset(32);

type ExtractRESET<T> = T extends WritableAtom<unknown, infer U>
  ? U extends Symbol
    ? U
    : never
  : never;

type TypeOfRESET = ExtractRESET<typeof tempFahrenheitAtom>; // typeof RESET === TypeOfRESET

but the problem then becomes that there's no clear way to implement Recoils newValue instanceof DefaultValue as typeof newValue === "symbol" in the general case is not specific enough. This could be solved by adding isReset(newValue) that would be exported from jotai/utils and be just (js code) isReset = (value) => value === RESET. It could work. But I still don't see the reason why RESET can't be just a regular value like here https://codesandbox.io/s/jotai-atom-with-reset-bug-k6xcu?file=/src/atoms.ts

In a sense, if RESET is not exported it is easier to implement your own atomWithReset (and useResetAtom), instead of using atomWithReset from jotai/utils.

Having said all of that :) I only decided to try out atomWithReset to see if it could work in the same way as Recoil does. I have no real use case apart from this codepen example. This was just an exercise if I can use useResetAtom with atom that wasn't created by atomWithReset. My first inclination was that atomWithReset would be able to accept write parameter, or maybe even write and reset parameters like this:

export const tempCelciusAtom = atomWithReset(
  (get) => ((get(tempFahrenheitAtom) - 32) * 5) / 9,
  (get, set, newValue: number) => set(tempFahrenheitAtom, (newValue * 9) / 5 + 32),
  (get, set, resetValue) => set(tempFahrenheitAtom, resetValue)
);

@JLarky
Copy link
Contributor Author

JLarky commented Nov 30, 2020

I similar to TypeOfRESET fashion it's possible to get RESET symbol value anyway :)

const RESET = await getReset();

const getReset = async () => {
   let RESET: TypeOfRESET;
   const captureAtom = atom(() => "", (get, set, resetValue) => {RESET = resetValue});
   // create React root
   // use useResetRecoilState(captureAtom) there
   // await for write function of captureAtom to trigger
   return RESET
}

@dai-shi
Copy link
Member

dai-shi commented Nov 30, 2020

As for typing, does this work?

type ExtractAtomUpdate<T> = T extends WritableAtom<unknown, infer U> ? U : never

type TypeOfRESET = Exclude<ExtractAtomUpdate<typeof tempFahrenheitAtom>, number>

Having said all of that :) I only decided to try out atomWithReset to see if it could work in the same way as Recoil does. I have no real use case apart from this codepen example.

OK, let's keep this issue open for a while to gather more use cases, and then consider what to do.

My first inclination was that atomWithReset would be able to accept write parameter

I don't think atomWithReset needs to cover all use cases. One can create their own version of it with the bare atom(). I think the current version works pretty well for primitive atom use cases.

That said, I'm happy to have atomWithReset improved if we can do it nicely. isReset could be an option.

@JLarky
Copy link
Contributor Author

JLarky commented Nov 30, 2020

OK, let's keep this issue open for a while to gather more use cases, and then consider what to do.

makes sense.

I don't think atomWithReset needs to cover all use cases.

I got to the same conclusion once I looked more into it :)

One can create their own version of it with the bare atom()

I agree with that :) it's just a shame that you would have to reimplement useResetAtom as well.

isReset could be an option

I think a solution that gets us 90% there is to export RESET type (note that it's no longer typeof RESET).

import { atom } from "jotai";
import { atomWithReset, RESET } from "jotai/utils";

export const tempFahrenheitAtom = atomWithReset(32);
export const tempCelciusAtom = atom(
  (get) => ((get(tempFahrenheitAtom) - 32) * 5) / 9,
  (get, set, newValue: number | RESET) =>
    set(
      tempFahrenheitAtom,
      typeof newValue === "symbol" ? newValue : (newValue * 9) / 5 + 32
    )
);

// in component
const reset = useResetAtom(tempCelciusAtom);

and adding isReset will get us 99% there:

import { atom } from "jotai";
import { atomWithReset, RESET, isReset } from "jotai/utils";

export const tempFahrenheitAtom = atomWithReset(32);
export const tempCelciusAtom = atom(
  (get) => ((get(tempFahrenheitAtom) - 32) * 5) / 9,
  (get, set, newValue: number | RESET) =>
    set(
      tempFahrenheitAtom,
      isReset(newValue) ? newValue : (newValue * 9) / 5 + 32
    )
);

// in component
const reset = useResetAtom(tempCelciusAtom);

when I'm talking about getting 100% there I mean something like this:

import { atom } from "jotai";
import { atomWithReset, RESET } from "jotai/utils";

export const tempFahrenheitAtom = atomWithReset(32);
export const tempCelciusAtom = atom(
  (get) => ((get(tempFahrenheitAtom) - 32) * 5) / 9,
  (get, set, newValue: number | typeof RESET) =>
    set(
      tempFahrenheitAtom,
      newValue > 100 ? RESET : (newValue * 9) / 5 + 32
    )
);

// in component
const [, set] = useAtom(tempCelciusAtom);
set(999);

which would be the same as Recoil's:

newValue > 100 ? (new DefaultValue()) : (newValue * 9) / 5 + 32

Returning back to the extensibility of atomWithReset, since resetting in Recoil is the first-class citizen you can actually do this:

export const tempCelcius = selector<number>({
  key: "tempCelcius",
  get: ({ get }) => ((get(tempFahrenheit) - 32) * 5) / 9,
  set: ({ set, reset }, newValue) =>
    newValue instanceof DefaultValue
      ? reset(tempFahrenheit)
      : set(tempFahrenheit, (newValue * 9) / 5 + 32)
});

so last piece of the puzzle could be exporting type of RESET, isAtom and reset:

import { atom } from "jotai";
import { atomWithReset, RESET, isReset, reset } from "jotai/utils";

export const tempFahrenheitAtom = atomWithReset(32);
export const tempCelciusAtom = atom(
  (get) => ((get(tempFahrenheitAtom) - 32) * 5) / 9,
  (get, set, newValue: number | RESET) =>
    isReset(newValue)
      ? reset(set, tempFahrenheitAtom)
      : set(tempFahrenheitAtom, (newValue * 9) / 5 + 32)
);

// in component
const reset = useResetAtom(tempCelciusAtom);

but at what point we could agree that this is over-engineering and it's easier to just export the RESET symbol? :-D

@dai-shi
Copy link
Member

dai-shi commented Nov 30, 2020

wow, your consideration is really thorough. It's clear that you make good points.
It seems like we have two different approaches: a) provide something minimal, b) provide something recoil compatible.
Both are fine with jotai/utils. We could provide two variants.
Contradictory to my first comment, I'm more convinced about exporting the reset symbol.

@SpenserJ
Copy link

I just ran into a similar need for resetting an atom through the setter, and having access to either the RESET symbol or a reset function (which I also agree is overkill) would be extremely valuable. In my situation, I have a few pieces of state that are not dependent on each other in the usual sense, but only one can be set at a time for filtering. Each of these are defined in separate files, as there are other atoms that depend on them but aren't relevant for this example, otherwise it would be easy to just duplicate the initial value.

const searchAtom = atomWithReset('');
const timelineAtom = atomWithReset(0);
const categoryAtom = atomWithReset('random');

// The value is used in a single component for a GraphQL query, while the set is used from multiple components.
// Only one filter can be set at a time, but they are stored in separate atoms to reduce renders since multiple
// components will depend on each of the individual atoms, and so that derived atoms don't need to know
// about the other types of filters
const filterAtom = atom(
  (get) => {
    if (get(searchAtom)) { return { search: get(searchAtom) }; }
    if (get(timelineAtom)) { return { timeline: get(timelineAtom) }; }
    return { category: get(categoryAtom) };
  },
  (get, set, [type, value]) => {
    set(searchAtom, type === 'search' ? value : RESET);
    set(timelineAtom, type === 'timeline' ? value : RESET);
    set(categoryAtom, type === 'category' ? value : RESET);
  },
);

// In the search component
const [searchValue] = useAtom(searchAtom);
const setFilter = useUpdateAtom(filterAtom);
...
setFilter(['search', 'some value']);

// In the component that runs the query
const [filterValue] = useAtom(filterAtom); // { search: 'some value' };

This setup would allow my components to use the search atom's value (or atoms that depend on it) and only rerender when that atom changes, while the filterAtom would allow the query to get the final value and components to set the appropriate atom and clear the others. All that it would require is an export for the RESET symbol

@dai-shi
Copy link
Member

dai-shi commented Nov 30, 2020

@SpenserJ Thanks for chiming in.

As the use case is clear, and users are not confused, exporting RESET would be good. (better than exporting type only, which not only limits the usage, but doesn't make sense.)

@dai-shi dai-shi linked a pull request Nov 30, 2020 that will close this issue
JLarky added a commit to JLarky/jotai that referenced this issue Dec 4, 2020
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 a pull request may close this issue.

3 participants