Feat: ability to validate atoms , `validateAtoms` #5

barelyhuman · 2022-07-02T13:06:30Z

Adds the validateAtoms implementation for overall form atom validation.

Usage example:

import { atomWithValidate, validateAtoms } from "jotai-form";

const nameAtom = atomWithValidate("foo", {
  validate: (v) => v, // individual atom validation (nothing here just for this example)
});

const ageAtom = atomWithValidate(18, {
  validate: (v) => v, // individual atom validation (nothing here just for this example)
});

const formStateAtom = validateAtoms(
  {
    name: nameAtom,
    age: ageAtom,
  },
  async (values) => {
   // validating the entire atom group for a specific form
    await Yup.object()
      .shape({
        name: Yup.string().min(3).required(),
        age: Yup.number().min(18).required(),
      })
      .validate(values);
  }
);

export const Component = () => {
  const [name, setName] = useAtom(nameAtom);
  const [age, setAge] = useAtom(ageAtom);
  const [formState] = useAtom(formStateAtom);

  return (
    <>
      <input
        placeholder="Name"
        value={name.value}
        onChange={(e) => setName(e.target.value)}
      />
      <input
        type="number"
        placeholder="Age"
        value={age.value}
        onChange={(e) => setAge(parseInt(e.target.value,10))}
      />
      <button type="submit" disabled={!formState.isValid}>Submit</button>
    </>
  );
};

barelyhuman · 2022-07-02T13:07:33Z

@dai-shi there is a ts-expect statement in the code that I'd like you to check. I think it's from the official jotai libraries expectation of the Getter type but would need confirmation

dai-shi · 2022-07-03T00:03:13Z

Types are fixed. You can't have a union type for getter. I'm not sure if it's possible to improve.

src/index.ts

will change the API to avoid adding a helper to reduce developer work for basic interaction

dai-shi

quick review

src/validateAtoms.ts

remove the un-needed types from the API change and also make the validator function consistent with atomWithValidate

dai-shi

Not sure the explanation is enough, but let it go.

src/validateAtoms.ts

barelyhuman · 2022-07-04T07:14:09Z

Do not merge even if it suffices, have to write tests for async validation

dai-shi

other than that, the code looks okay.

dai-shi · 2022-07-05T09:09:59Z

src/validateAtoms.ts

+
+export type Validator = <Values extends Record<string, unknown>>(
+  values: Values,
+) => Promise<unknown>;


Maybe this should be unknown | Promise<unknown> or void | Promise<void>. We support sync validator, right?

that totally depends on the loadable API but I think the sync one should be working, i'll check in the evening

done.
Though, there's an issue with using mixed typed atoms right now. The types for validator and labelled atoms needs to be fixed for that.

On it next.

will add sync validator function in types and adds a test trying it out with an async test library

will infer the atom's internal generic and use that to generate the type sent to the validator

dai-shi

final review.

src/validateAtoms.ts

dai-shi · 2022-07-07T12:10:11Z

src/validateAtoms.ts

+  const $getSourceAtomVals = (get: Getter) => {
+    const values = Object.fromEntries(
+      Object.entries(labeledAtoms).map(([k, v]) => {
+        const atomValue = get(v);
+        return [k, atomValue.value];
+      }),
+    );
+    return values as Record<Keys, inferGeneric<Vals>>;
+  };


I thinks it's better to make this an atom.

Suggested change

const $getSourceAtomVals = (get: Getter) => {

const values = Object.fromEntries(

Object.entries(labeledAtoms).map(([k, v]) => {

const atomValue = get(v);

return [k, atomValue.value];

}),

);

return values as Record<Keys, inferGeneric<Vals>>;

};

const valsAtom = atom((get) => {

const values = Object.fromEntries(

Object.entries(labeledAtoms).map(([k, v]) => {

const atomValue = get(v);

return [k, atomValue.value];

}),

);

return values as Record<Keys, inferGeneric<Vals>>;

});

too many listeners on the store?
cause it's being used in 2 places as a simple helper right now

const valsAtom = atom((get: Getter) => { const values = Object.fromEntries( Object.entries(labeledAtoms).map(([k, v]) => { const atomValue = get(v); return [k, atomValue.value]; }), ); return values as Record<Keys, inferGeneric<Vals>>; }); const baseAtom = atom(async (get) => { // extract value from each atom and assign to the given key as label return validator(get(valsAtom)); }); const normalizerAtom = atom((get) => { const values = get(valsAtom); const state = get(loadable(baseAtom)); return { ...state, values, }; }); const derv = atom((get) => { const validatorState = get(normalizerAtom); //.... other code })

valsAtom can be more efficient because it caches the result, than a simple helper.

makes sense, pushed

src/validateAtoms.ts

dai-shi · 2022-07-07T23:20:26Z

src/validateAtoms.ts

+  });
+
+  const derv = atom((get) => {
+    const validatorState = get(normalizerAtom);


Suggested change

const validatorState = get(normalizerAtom);

const values = get(valsAtom);

const state = get(loadable(baseAtom));

const validatorState = {

...state,

values,

};

Does this work?

the test's say it's working,
that's nice, 1 listener reduced.

pushed with other simplifications

- remove un-needed variables - use switch instead to avoid case checks

dai-shi

LGTM

Please update the PR description.

Feel free to merge it and ship it.

BenBeattieHood · 2022-07-10T23:04:23Z

Oh wow! I go away for a break, I come back and you've solved this 🤩 Love your work, thank you 👍

base implementation

6dbf2cc

barelyhuman and others added 4 commits July 2, 2022 19:04

rename and add to entry file

a084e3b

test: add basic form validation

f2fb8c8

fix types

afbcbfd

simplify types

4644e2c

dai-shi reviewed Jul 3, 2022

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

refactor: validateAtoms API

120f87b

will change the API to avoid adding a helper to reduce developer work for basic interaction

dai-shi reviewed Jul 3, 2022

View reviewed changes

src/validateAtoms.ts Outdated Show resolved Hide resolved

src/validateAtoms.ts Outdated Show resolved Hide resolved

src/validateAtoms.ts Show resolved Hide resolved

fix: remove types, consistent validator function

2c02a50

remove the un-needed types from the API change and also make the validator function consistent with atomWithValidate

dai-shi reviewed Jul 3, 2022

View reviewed changes

src/validateAtoms.ts Outdated Show resolved Hide resolved

src/validateAtoms.ts Outdated Show resolved Hide resolved

add values into the atom

a51e9b8

dai-shi marked this pull request as draft July 5, 2022 09:08

dai-shi approved these changes Jul 5, 2022

View reviewed changes

barelyhuman added 2 commits July 6, 2022 20:34

chore: type simplification and test

77a90e0

will add sync validator function in types and adds a test trying it out with an async test library

fix: infer generic types

3c27421

will infer the atom's internal generic and use that to generate the type sent to the validator

dai-shi marked this pull request as ready for review July 7, 2022 12:05

dai-shi reviewed Jul 7, 2022

View reviewed changes

refac: value getter to atom

f821f98

dai-shi reviewed Jul 7, 2022

View reviewed changes

simplify

826ef0b

- remove un-needed variables - use switch instead to avoid case checks

dai-shi approved these changes Jul 8, 2022

View reviewed changes

barelyhuman changed the title ~~validateForm~~ Feat: ability to validate atoms , validateAtoms Jul 8, 2022

barelyhuman merged commit ace612e into main Jul 8, 2022

barelyhuman mentioned this pull request Jul 10, 2022

Form-level validation #3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: ability to validate atoms , `validateAtoms` #5

Feat: ability to validate atoms , `validateAtoms` #5

barelyhuman commented Jul 2, 2022 •

edited

Loading

barelyhuman commented Jul 2, 2022

dai-shi commented Jul 3, 2022

dai-shi left a comment

dai-shi left a comment

barelyhuman commented Jul 4, 2022 •

edited

Loading

dai-shi left a comment

dai-shi Jul 5, 2022

barelyhuman Jul 5, 2022

barelyhuman Jul 6, 2022

dai-shi left a comment

dai-shi Jul 7, 2022

barelyhuman Jul 7, 2022

dai-shi Jul 7, 2022

barelyhuman Jul 7, 2022

dai-shi Jul 7, 2022

barelyhuman Jul 8, 2022

barelyhuman Jul 8, 2022

dai-shi left a comment

BenBeattieHood commented Jul 10, 2022

-    const validatorState = get(normalizerAtom);
+    const values = get(valsAtom);
+    const state = get(loadable(baseAtom));
+    const validatorState = {
+      ...state,
+      values,
+    };

Feat: ability to validate atoms , validateAtoms #5

Feat: ability to validate atoms , validateAtoms #5

Conversation

barelyhuman commented Jul 2, 2022 • edited Loading

barelyhuman commented Jul 2, 2022

dai-shi commented Jul 3, 2022

dai-shi left a comment

Choose a reason for hiding this comment

dai-shi left a comment

Choose a reason for hiding this comment

barelyhuman commented Jul 4, 2022 • edited Loading

dai-shi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dai-shi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dai-shi left a comment

Choose a reason for hiding this comment

BenBeattieHood commented Jul 10, 2022

Feat: ability to validate atoms , `validateAtoms` #5

Feat: ability to validate atoms , `validateAtoms` #5

barelyhuman commented Jul 2, 2022 •

edited

Loading

barelyhuman commented Jul 4, 2022 •

edited

Loading