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

Is atomFamily supported? #33

Closed
martinhath opened this issue Jul 29, 2024 · 6 comments
Closed

Is atomFamily supported? #33

martinhath opened this issue Jul 29, 2024 · 6 comments

Comments

@martinhath
Copy link
Contributor

The atomFamily utility is a useful function, but it seems that swc-jotai either doesn't support it altogether, or that it is buggy. Tests doesn't mention it, and I tried to write a quick test:

    test_inline!(
        Syntax::default(),
        |_| transform(None, None),
        handle_normal_export,
        r#"
import { atom } from "jotai";
export const one = atom(0);
export const two = atomFamily(() => atom(1));"#,
        r#"
globalThis.jotaiAtomCache = globalThis.jotaiAtomCache || {
  cache: new Map(),
  get(name, inst) { 
    if (this.cache.has(name)) {
      return this.cache.get(name)
    }
    this.cache.set(name, inst)
    return inst
  },
}
import { atom } from "jotai";
export const one = globalThis.jotaiAtomCache.get("atoms.ts/one", atom(0));
export const two = globalThis.jotaiAtomCache.get("atoms.ts/two", atomFamily(() => atom(1)));
"#
    );

which outputs

 globalThis.jotaiAtomCache = globalThis.jotaiAtomCache || {
     cache: new Map(),
     get (name, inst) {
         if (this.cache.has(name)) {
             return this.cache.get(name);
         }
         this.cache.set(name, inst);
         return inst;
     }
 };
 import { atom } from "jotai";
 export const one = globalThis.jotaiAtomCache.get("atoms.ts/one", atom(0));
<export const two = globalThis.jotaiAtomCache.get("atoms.ts/two", atomFamily(()=>atom(1)));
>const two = globalThis.jotaiAtomCache.get("atoms.ts/two", atom(1));

Two things:

  1. export is removed for some reason
  2. the value in .get is just atom(1) and nothing else
    First time looking at the codebase just now, so not sure if this is intentional or what, but it looks like a bug.
@martinhath
Copy link
Contributor Author

martinhath commented Jul 30, 2024

Have done some more digging.

atomFamily seems like it should be supported, as it's in the ATOM_IMPORTS list in common/src/constants.rs. Weird that it's getting omitted here (or that the inserted code is just the atom), but that looks like a bug now.

Here's what the Babel plugin outputs for the same input as the test above:

export const one = globalThis.jotaiAtomCache.get("/Users/martin/src/state-management/src/jotai.tsx/one", atom(0));
one.debugLabel = "one";
export const two = globalThis.jotaiAtomCache.get("/Users/martin/src/state-management/src/jotai.tsx/two", atomFamily(() => atom(1)));
two.debugLabel = "two";

so presumably the test case I wrote should pass.

@dai-shi
Copy link
Member

dai-shi commented Jul 30, 2024

I wonder if the babel plugin version works as expected. atomFamily(...) doesn't return an atom.

To make it work, we would probably need getParams implemented in the core. pmndrs/jotai#2679

@martinhath
Copy link
Contributor Author

It seems like it does; I wrote a quick test for it, in order to better understand how the hot-reload persisting was supposed to work. Basically this:

const testFamily = atomFamily((n: number) => atom(0));
const Buttons = () => {
  const [v0, set0] = useAtom(testFamily(0));
  const [v1, set1] = useAtom(testFamily(1));
  const [v2, set2] = useAtom(testFamily(2));
  return (
    <>
      <button onClick={() => set0((c) => c + 1)}>{v0}</button>
      <button onClick={() => set1((c) => c + 1)}>{v1}</button>
      <button onClick={() => set2((c) => c + 1)}>{v2}</button>
    </>
  );
};

The buttons increment their labels when clicked, and the labels persist through hot-reloads.

@martinhath
Copy link
Contributor Author

martinhath commented Jul 30, 2024

Update on the test case: adding import { atomFamily } from "jotai/utils"; to the test (and its output) makes the test pass.

@martinhath
Copy link
Contributor Author

I hadn't understood that I needed to add the names of my custom atom wrappers to the atomNames list in the config, as written in the README. Doing this helped.

However, I also had to change some of my "higher order atom" functions, from

export const atomFamily2 = <In, Out, At extends Atom<Out>>(fn: (i: In) => At) =>
  atomFamily(fn, deepEqual);

which outputs (note the missing export and the wrong .debugLabel)

const atomFamily2 = globalThis.jotaiAtomCache.get("/Users/martin/src/albatross/src/utils/jotai.ts/atomFamily2", atomFamily(fn, deepEqual));
atomFamily2.debugLabel = "atomFamily2";

to

export const atomFamily2 = <In, Out, At extends Atom<Out>>(
  fn: (i: In) => At,
) => {
  const af = atomFamily(fn, deepEqual);
  return af;
};

which outputs

export const atomFamily2 = (fn)=>{
    const af = atomFamily(fn, deepEqual);
    af.debugLabel = "af";
    return af;
};

@martinhath
Copy link
Contributor Author

I'll close this, since #36 is a better reproduction of the problems I had.

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

No branches or pull requests

2 participants