feat(solid-effect-atom)!: v0.3.0 rewrite#39
Conversation
… (v0.3.0) BREAKING CHANGE: Complete rewrite of the library to wrap @effect-atom/atom. Adapts the API to be compatible with Effect v3 and provides SolidJS bindings similar to effect-smol.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpgrades Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Component as Component
participant Provider as RegistryProvider
participant Registry as AtomRegistry
participant Atom as Atom
participant EffectLib as "@effect-atom/atom"
Component->>Provider: mount (wrap UI with RegistryProvider)
Provider->>Registry: create registry (AtomRegistry.make(...))
Component->>Registry: call hook (useAtomValue / useAtomSet / useAtomSubscribe / useAtomRef)
Registry->>Atom: registry.get / subscribe
Atom->>EffectLib: read/write operations
EffectLib-->>Atom: return value/result
Registry-->>Component: notify / accessor update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@apps/docs/src/content/docs/es/solid/packages/solid-effect-atom.md`:
- Around line 169-180: The API reference list is missing two hooks; add entries
for useAtomRefProp and useAtomRefPropValue to the Hooks section so it matches
the English docs: include lines for **`useAtomRefProp(ref, prop)`** (or similar
brief description) and **`useAtomRefPropValue(ref, prop?)`** describing what
they return (accessor/setter or accessor), placing them alongside the other
hooks (e.g., after useAtomRef) so the Spanish docs are consistent with the
reference.
In `@apps/docs/src/content/docs/solid/packages/solid-effect-atom.md`:
- Around line 169-180: The API reference list omits the hooks useAtomRefProp and
useAtomRefPropValue; update the Hooks section (where useAtom, useAtomValue, etc.
are listed) to include entries for useAtomRefProp and useAtomRefPropValue so the
docs match the referenced hooks; ensure the new bullets follow the same style as
the others (function name with short return description) and reference the exact
symbols useAtomRefProp and useAtomRefPropValue so they’re discoverable in the
list.
In `@apps/solid-example/src/components/Header.tsx`:
- Line 84: Update the navigation label string in the Header component to correct
the typo: change the span text that currently reads "Start -
`@effecitfy/solid-effect-atom` Atom Demo" to use the correct library name
"@effectify" (i.e., "Start - `@effectify/solid-effect-atom` Atom Demo") so the
displayed navigation label matches the real package name.
In `@apps/solid-example/src/routes/atom-demo.tsx`:
- Around line 35-46: Several button elements across the components (Counter,
RefreshDemo, AtomRefDemo, SetOnlyDemo, SubscribeDemo) are missing an explicit
type and default to "submit", potentially causing unintended form submissions;
update every <button> used for non-form-submission actions (e.g., the
Increment/Reset buttons using setCount, and other interactive buttons in the
listed components) to include type="button" so they behave as intended and do
not trigger form submits.
- Line 157: The MountDemo component declares an unused signal pair `mounted` and
`setMounted` created via `createSignal(false)`; remove the unused declaration to
clean up dead code—delete the `const [mounted, setMounted] =
createSignal(false)` line and any related unused references (if any) in the
`MountDemo` component so only active signals remain.
In `@packages/solid/effect-atom/src/hooks.ts`:
- Around line 62-95: The setAtom function currently uses multiple unsafe casts
("as any") to satisfy its conditional return type; refactor by splitting the
implementation into explicit, type-safe variants (e.g., setAtomValue,
setAtomPromise, setAtomPromiseExit) or by providing properly typed overloads for
setAtom so the runtime branches returned from the
registry.set/Effect.runPromiseExit + AtomRegistry.getResult paths have concrete
types instead of any; also rename flattenExit to a throwing-explicit name such
as unwrapExitOrThrow (and update its signature and call sites including where
Effect.runPromiseExit is .then(flattenExit)) so the throwing behavior is clear.
- Around line 192-214: useAtomRefProp is named like a Solid hook but merely
returns ref.prop(prop) without using Solid primitives; either rename
useAtomRefProp to getAtomRefProp (or remove the "use" prefix) wherever it's
exported/used, or implement hook semantics by wrapping the returned AtomRef in a
Solid primitive (e.g., createMemo or createSignal) so it follows
lifecycle/reactivity expectations; update the implementation referenced by
useAtomRefProp and ensure useAtomRefPropValue still composes correctly with the
renamed/adjusted function (useAtomRefProp, useAtomRefPropValue, and ref.prop are
the symbols to update).
- Around line 41-56: The hook useAtomValue currently throws when RegistryContext
is missing while other hooks (useAtomInitialValues, useAtomMount,
useAtomSubscribe) return silently—make behavior consistent by either throwing in
all hooks or returning null/undefined in all; update useAtomValue or the other
hooks to match your chosen convention and document it. Also fix the unsafe cast
in createAtomAccessor: align the types between AtomRegistry.Registry.subscribe
and the Solid signal setter by updating subscribe's callback type (or adding a
properly typed adapter) so you can pass setValue without using "as any" (refer
to createAtomAccessor, registry.subscribe, setValue, AtomRegistry.Registry,
useAtomValue).
- Around line 21-35: Remove the unreachable null-check and unsafe cast in
useAtomInitialValues: delete the early return that guards on registry after
calling useContext(RegistryContext) since RegistryContext always provides a
Registry, and replace the casted call "(registry as any).set(atom, value)" with
the properly typed call "registry.set(atom, value)" so the function uses the
AtomRegistry.Registry type directly; ensure initialValuesSet and registry are
used as before and no other type widening is introduced.
| ## Referencia de API | ||
|
|
||
| ### Hooks | ||
|
|
||
| - **`useAtom(atom)`**: Retorna `[accessor, setter]`. | ||
| - **`useAtomValue(atom, selector?)`**: Retorna `accessor`. | ||
| - **`useAtomSet(atom)`**: Retorna solo el `setter`. | ||
| - **`useAtomSubscribe(atom, callback)`**: Se suscribe a cambios. | ||
| - **`useAtomMount(atom)`**: Monta el átomo en el registro. | ||
| - **`useAtomInitialValues(values)`**: Inicializa átomos en el registro actual. | ||
| - **`useAtomRefresh(atom)`**: Retorna una función para refrescar el átomo. | ||
| - **`useAtomRef(ref)`**: Se suscribe a un `AtomRef`. |
There was a problem hiding this comment.
API Reference list is missing useAtomRefProp and useAtomRefPropValue.
Same as the English documentation - the API reference list should include useAtomRefProp and useAtomRefPropValue for consistency with the reference documentation.
📝 Suggested addition
- **`useAtomRefresh(atom)`**: Retorna una función para refrescar el átomo.
- **`useAtomRef(ref)`**: Se suscribe a un `AtomRef`.
+- **`useAtomRefProp(ref, prop)`**: Crea un `AtomRef` derivado para una propiedad específica.
+- **`useAtomRefPropValue(ref, prop)`**: Se suscribe a una propiedad específica de un `AtomRef`.🧰 Tools
🪛 LanguageTool
[grammar] ~176-~176: Elimina la puntuación
Context: ...- useAtomSubscribe(atom, callback): Se suscribe a cambios. - **`useAtomMoun...
(QB_NEW_ES_OTHER_ERROR_IDS_UNNECESSARY_PUNCTUATION)
[grammar] ~177-~177: Elimina la puntuación
Context: ...be a cambios. - useAtomMount(atom): Monta el átomo en el registro. - **`use...
(QB_NEW_ES_OTHER_ERROR_IDS_UNNECESSARY_PUNCTUATION)
[grammar] ~179-~179: Elimina la puntuación
Context: ...tro actual. - useAtomRefresh(atom): Retorna una función para refrescar el á...
(QB_NEW_ES_OTHER_ERROR_IDS_UNNECESSARY_PUNCTUATION)
[grammar] ~180-~180: Elimina la puntuación
Context: ...rescar el átomo. - useAtomRef(ref): Se suscribe a un AtomRef. ### Compon...
(QB_NEW_ES_OTHER_ERROR_IDS_UNNECESSARY_PUNCTUATION)
🤖 Prompt for AI Agents
In `@apps/docs/src/content/docs/es/solid/packages/solid-effect-atom.md` around
lines 169 - 180, The API reference list is missing two hooks; add entries for
useAtomRefProp and useAtomRefPropValue to the Hooks section so it matches the
English docs: include lines for **`useAtomRefProp(ref, prop)`** (or similar
brief description) and **`useAtomRefPropValue(ref, prop?)`** describing what
they return (accessor/setter or accessor), placing them alongside the other
hooks (e.g., after useAtomRef) so the Spanish docs are consistent with the
reference.
| ## API Reference | ||
|
|
||
| ### Hooks | ||
|
|
||
| - **`useAtom(atom)`**: Returns `[accessor, setter]`. | ||
| - **`useAtomValue(atom, selector?)`**: Returns `accessor`. | ||
| - **`useAtomSet(atom)`**: Returns only the `setter`. | ||
| - **`useAtomSubscribe(atom, callback)`**: Subscribes to changes. | ||
| - **`useAtomMount(atom)`**: Mounts the atom in the registry. | ||
| - **`useAtomInitialValues(values)`**: Initializes atoms in the current registry. | ||
| - **`useAtomRefresh(atom)`**: Returns a function to refresh the atom. | ||
| - **`useAtomRef(ref)`**: Subscribes to an `AtomRef`. |
There was a problem hiding this comment.
API Reference list is missing useAtomRefProp and useAtomRefPropValue.
The API reference docs at solid/reference/solid-effect-atom.md document useAtomRefProp and useAtomRefPropValue hooks, but they are not listed here. Consider adding them for consistency.
📝 Suggested addition
- **`useAtomRefresh(atom)`**: Returns a function to refresh the atom.
- **`useAtomRef(ref)`**: Subscribes to an `AtomRef`.
+- **`useAtomRefProp(ref, prop)`**: Creates a derived `AtomRef` for a specific property.
+- **`useAtomRefPropValue(ref, prop)`**: Subscribes to a specific property of an `AtomRef`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## API Reference | |
| ### Hooks | |
| - **`useAtom(atom)`**: Returns `[accessor, setter]`. | |
| - **`useAtomValue(atom, selector?)`**: Returns `accessor`. | |
| - **`useAtomSet(atom)`**: Returns only the `setter`. | |
| - **`useAtomSubscribe(atom, callback)`**: Subscribes to changes. | |
| - **`useAtomMount(atom)`**: Mounts the atom in the registry. | |
| - **`useAtomInitialValues(values)`**: Initializes atoms in the current registry. | |
| - **`useAtomRefresh(atom)`**: Returns a function to refresh the atom. | |
| - **`useAtomRef(ref)`**: Subscribes to an `AtomRef`. | |
| ## API Reference | |
| ### Hooks | |
| - **`useAtom(atom)`**: Returns `[accessor, setter]`. | |
| - **`useAtomValue(atom, selector?)`**: Returns `accessor`. | |
| - **`useAtomSet(atom)`**: Returns only the `setter`. | |
| - **`useAtomSubscribe(atom, callback)`**: Subscribes to changes. | |
| - **`useAtomMount(atom)`**: Mounts the atom in the registry. | |
| - **`useAtomInitialValues(values)`**: Initializes atoms in the current registry. | |
| - **`useAtomRefresh(atom)`**: Returns a function to refresh the atom. | |
| - **`useAtomRef(ref)`**: Subscribes to an `AtomRef`. | |
| - **`useAtomRefProp(ref, prop)`**: Creates a derived `AtomRef` for a specific property. | |
| - **`useAtomRefPropValue(ref, prop)`**: Subscribes to a specific property of an `AtomRef`. |
🤖 Prompt for AI Agents
In `@apps/docs/src/content/docs/solid/packages/solid-effect-atom.md` around lines
169 - 180, The API reference list omits the hooks useAtomRefProp and
useAtomRefPropValue; update the Hooks section (where useAtom, useAtomValue, etc.
are listed) to include entries for useAtomRefProp and useAtomRefPropValue so the
docs match the referenced hooks; ensure the new bullets follow the same style as
the others (function name with short return description) and reference the exact
symbols useAtomRefProp and useAtomRefPropValue so they’re discoverable in the
list.
| }} | ||
| > | ||
| <Globe size={20} /> | ||
| <span class="font-medium">Start - @effecitfy/solid-effect-atom Atom Demo</span> |
There was a problem hiding this comment.
Typo in navigation label: "@effecitfy" should be "@effectify".
The library name is misspelled in the navigation menu.
✏️ Proposed fix
- <span class="font-medium">Start - `@effecitfy/solid-effect-atom` Atom Demo</span>
+ <span class="font-medium">Start - `@effectify/solid-effect-atom` Atom Demo</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span class="font-medium">Start - @effecitfy/solid-effect-atom Atom Demo</span> | |
| <span class="font-medium">Start - `@effectify/solid-effect-atom` Atom Demo</span> |
🤖 Prompt for AI Agents
In `@apps/solid-example/src/components/Header.tsx` at line 84, Update the
navigation label string in the Header component to correct the typo: change the
span text that currently reads "Start - `@effecitfy/solid-effect-atom` Atom Demo"
to use the correct library name "@effectify" (i.e., "Start -
`@effectify/solid-effect-atom` Atom Demo") so the displayed navigation label
matches the real package name.
| <button | ||
| class="px-4 py-2 bg-cyan-500 hover:bg-cyan-600 text-white font-semibold rounded-lg transition-colors" | ||
| onClick={() => setCount((c) => c + 1)} | ||
| > | ||
| Increment | ||
| </button> | ||
| <button | ||
| class="px-4 py-2 bg-slate-600 hover:bg-slate-700 text-white font-semibold rounded-lg transition-colors" | ||
| onClick={() => setCount(0)} | ||
| > | ||
| Reset | ||
| </button> |
There was a problem hiding this comment.
Add explicit type="button" to buttons.
All interactive buttons in this file lack an explicit type attribute. The default type is submit, which can cause unintended form submissions. This applies to buttons in Counter, RefreshDemo, AtomRefDemo, SetOnlyDemo, and SubscribeDemo components.
✏️ Example fix for Counter buttons (apply similar pattern to others)
<button
+ type="button"
class="px-4 py-2 bg-cyan-500 hover:bg-cyan-600 text-white font-semibold rounded-lg transition-colors"
onClick={() => setCount((c) => c + 1)}
>
Increment
</button>
<button
+ type="button"
class="px-4 py-2 bg-slate-600 hover:bg-slate-700 text-white font-semibold rounded-lg transition-colors"
onClick={() => setCount(0)}
>
Reset
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| class="px-4 py-2 bg-cyan-500 hover:bg-cyan-600 text-white font-semibold rounded-lg transition-colors" | |
| onClick={() => setCount((c) => c + 1)} | |
| > | |
| Increment | |
| </button> | |
| <button | |
| class="px-4 py-2 bg-slate-600 hover:bg-slate-700 text-white font-semibold rounded-lg transition-colors" | |
| onClick={() => setCount(0)} | |
| > | |
| Reset | |
| </button> | |
| <button | |
| type="button" | |
| class="px-4 py-2 bg-cyan-500 hover:bg-cyan-600 text-white font-semibold rounded-lg transition-colors" | |
| onClick={() => setCount((c) => c + 1)} | |
| > | |
| Increment | |
| </button> | |
| <button | |
| type="button" | |
| class="px-4 py-2 bg-slate-600 hover:bg-slate-700 text-white font-semibold rounded-lg transition-colors" | |
| onClick={() => setCount(0)} | |
| > | |
| Reset | |
| </button> |
🧰 Tools
🪛 Biome (2.3.13)
[error] 35-38: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 41-44: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
In `@apps/solid-example/src/routes/atom-demo.tsx` around lines 35 - 46, Several
button elements across the components (Counter, RefreshDemo, AtomRefDemo,
SetOnlyDemo, SubscribeDemo) are missing an explicit type and default to
"submit", potentially causing unintended form submissions; update every <button>
used for non-form-submission actions (e.g., the Increment/Reset buttons using
setCount, and other interactive buttons in the listed components) to include
type="button" so they behave as intended and do not trigger form submits.
| export const useAtomInitialValues = (initialValues: Iterable<readonly [Atom.Atom<any>, any]>): void => { | ||
| const registry = useContext(RegistryContext) | ||
| if (!registry) return | ||
| let set = initialValuesSet.get(registry) | ||
| if (set === undefined) { | ||
| set = new WeakSet() | ||
| initialValuesSet.set(registry, set) | ||
| } | ||
| for (const [atom, value] of initialValues) { | ||
| if (!set.has(atom)) { | ||
| set.add(atom) | ||
| ;(registry as any).set(atom, value) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full context of the hooks.ts file to see RegistryContext definition
head -100 packages/solid/effect-atom/src/hooks.tsRepository: devx-op/effectify
Length of output: 3265
🏁 Script executed:
# Search for RegistryContext creation/definition in the file
rg -n "RegistryContext" packages/solid/effect-atom/src/hooks.ts -B2 -A2Repository: devx-op/effectify
Length of output: 2141
🏁 Script executed:
# Check for other usages of registry.set in this file, particularly around lines 78 and 88
rg -n "registry\.set" packages/solid/effect-atom/src/hooks.ts -B1 -A1Repository: devx-op/effectify
Length of output: 341
🏁 Script executed:
# Find the AtomRegistry or Registry type definition
rg -n "class AtomRegistry|interface.*Registry|type.*Registry" packages/solid/effect-atom/src/ --type ts -B1 -A10 | head -100Repository: devx-op/effectify
Length of output: 43
🏁 Script executed:
# Check the registry-context.js file to see if RegistryContext has a default value
cat packages/solid/effect-atom/src/registry-context.jsRepository: devx-op/effectify
Length of output: 143
🏁 Script executed:
# Also try .ts extension
find packages/solid/effect-atom/src -name "*registry-context*" -type fRepository: devx-op/effectify
Length of output: 111
🏁 Script executed:
# Search for AtomRegistry.Registry type definition
rg -n "export.*Registry|interface Registry|type Registry" packages/ --type ts -B2 -A8 | head -150Repository: devx-op/effectify
Length of output: 1562
🏁 Script executed:
# Now find the AtomRegistry.Registry interface to understand the set() signature
rg -n "interface Registry|class Registry" packages/ --type ts -B2 -A20 | head -200Repository: devx-op/effectify
Length of output: 43
🏁 Script executed:
# Also search in the `@effect-atom/atom` package specifically
find packages -path "*/atom/Registry*" -o -path "*/atom/registry*" | head -20Repository: devx-op/effectify
Length of output: 43
🏁 Script executed:
# Search for how registry.set is called across the codebase to understand the typing better
rg -n "registry\.set\|\.set\(atom" packages/solid/effect-atom/src/ --type ts -B2 -A2Repository: devx-op/effectify
Length of output: 43
🏁 Script executed:
# Check if there are type definition files that might define the set() method
find packages -name "*.d.ts" | head -20Repository: devx-op/effectify
Length of output: 132
🏁 Script executed:
# Search more broadly for set method definitions with atom parameter
rg -n "set\s*\(" packages/solid/effect-atom/src/ --type ts -B1 -A1 | head -150Repository: devx-op/effectify
Length of output: 981
Remove dead registry null check and unsafe type cast.
Two issues verified:
-
Dead code on line 23:
RegistryContextis created with a default value (createContext<AtomRegistry.Registry>(AtomRegistry.make())), souseContext(RegistryContext)will always return a Registry instance. The early return is unreachable and should be removed. -
Type cast on line 32: The cast
(registry as any).set(atom, value)bypasses type safety. In thesetAtomfunction (lines 78, 88),registry.set()is called without casting despite having the sameAtomRegistry.Registrytype. The cast should be removed and proper typing applied instead.
♻️ Suggested fix
export const useAtomInitialValues = (initialValues: Iterable<readonly [Atom.Atom<any>, any]>): void => {
const registry = useContext(RegistryContext)
- if (!registry) return
let set = initialValuesSet.get(registry)
if (set === undefined) {
set = new WeakSet()
initialValuesSet.set(registry, set)
}
for (const [atom, value] of initialValues) {
if (!set.has(atom)) {
set.add(atom)
- ;(registry as any).set(atom, value)
+ registry.set(atom as Atom.Writable<any, any>, value)
}
}
}🤖 Prompt for AI Agents
In `@packages/solid/effect-atom/src/hooks.ts` around lines 21 - 35, Remove the
unreachable null-check and unsafe cast in useAtomInitialValues: delete the early
return that guards on registry after calling useContext(RegistryContext) since
RegistryContext always provides a Registry, and replace the casted call
"(registry as any).set(atom, value)" with the properly typed call
"registry.set(atom, value)" so the function uses the AtomRegistry.Registry type
directly; ensure initialValuesSet and registry are used as before and no other
type widening is introduced.
| export const useAtomValue: { | ||
| <A>(atom: Atom.Atom<A>): Accessor<A> | ||
| <A, B>(atom: Atom.Atom<A>, f: (_: A) => B): Accessor<B> | ||
| } = <A>(atom: Atom.Atom<A>, f?: (_: A) => A): Accessor<A> => { | ||
| const registry = useContext(RegistryContext) | ||
| if (!registry) { | ||
| throw new Error("RegistryContext not found. Ensure you are using RegistryProvider.") | ||
| } | ||
| return createAtomAccessor(registry, f ? Atom.map(atom, f) : atom) | ||
| } | ||
|
|
||
| function createAtomAccessor<A>(registry: AtomRegistry.Registry, atom: Atom.Atom<A>): Accessor<A> { | ||
| const [value, setValue] = createSignal<A>(registry.get(atom)) | ||
| onCleanup(registry.subscribe(atom, setValue as any)) | ||
| return value | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent error handling and type cast in signal subscription.
-
Inconsistent behavior:
useAtomValuethrows when registry is missing (lines 46-48), whileuseAtomInitialValues,useAtomMount, anduseAtomSubscribesilently return. Choose a consistent approach across all hooks. -
Type cast on line 54:
setValue as anysuggests a type mismatch between the signal setter and the subscribe callback signature. This should be properly typed.
♻️ Suggested fix for type safety
function createAtomAccessor<A>(registry: AtomRegistry.Registry, atom: Atom.Atom<A>): Accessor<A> {
const [value, setValue] = createSignal<A>(registry.get(atom))
- onCleanup(registry.subscribe(atom, setValue as any))
+ onCleanup(registry.subscribe(atom, (v: A) => setValue(() => v)))
return value
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useAtomValue: { | |
| <A>(atom: Atom.Atom<A>): Accessor<A> | |
| <A, B>(atom: Atom.Atom<A>, f: (_: A) => B): Accessor<B> | |
| } = <A>(atom: Atom.Atom<A>, f?: (_: A) => A): Accessor<A> => { | |
| const registry = useContext(RegistryContext) | |
| if (!registry) { | |
| throw new Error("RegistryContext not found. Ensure you are using RegistryProvider.") | |
| } | |
| return createAtomAccessor(registry, f ? Atom.map(atom, f) : atom) | |
| } | |
| function createAtomAccessor<A>(registry: AtomRegistry.Registry, atom: Atom.Atom<A>): Accessor<A> { | |
| const [value, setValue] = createSignal<A>(registry.get(atom)) | |
| onCleanup(registry.subscribe(atom, setValue as any)) | |
| return value | |
| } | |
| export const useAtomValue: { | |
| <A>(atom: Atom.Atom<A>): Accessor<A> | |
| <A, B>(atom: Atom.Atom<A>, f: (_: A) => B): Accessor<B> | |
| } = <A>(atom: Atom.Atom<A>, f?: (_: A) => A): Accessor<A> => { | |
| const registry = useContext(RegistryContext) | |
| if (!registry) { | |
| throw new Error("RegistryContext not found. Ensure you are using RegistryProvider.") | |
| } | |
| return createAtomAccessor(registry, f ? Atom.map(atom, f) : atom) | |
| } | |
| function createAtomAccessor<A>(registry: AtomRegistry.Registry, atom: Atom.Atom<A>): Accessor<A> { | |
| const [value, setValue] = createSignal<A>(registry.get(atom)) | |
| onCleanup(registry.subscribe(atom, (v: A) => setValue(() => v))) | |
| return value | |
| } |
🤖 Prompt for AI Agents
In `@packages/solid/effect-atom/src/hooks.ts` around lines 41 - 56, The hook
useAtomValue currently throws when RegistryContext is missing while other hooks
(useAtomInitialValues, useAtomMount, useAtomSubscribe) return silently—make
behavior consistent by either throwing in all hooks or returning null/undefined
in all; update useAtomValue or the other hooks to match your chosen convention
and document it. Also fix the unsafe cast in createAtomAccessor: align the types
between AtomRegistry.Registry.subscribe and the Solid signal setter by updating
subscribe's callback type (or adding a properly typed adapter) so you can pass
setValue without using "as any" (refer to createAtomAccessor,
registry.subscribe, setValue, AtomRegistry.Registry, useAtomValue).
| function setAtom<R, W, Mode extends "value" | "promise" | "promiseExit" = never>( | ||
| registry: AtomRegistry.Registry, | ||
| atom: Atom.Writable<R, W>, | ||
| options?: { | ||
| readonly mode?: ([R] extends [Result.Result<any, any>] ? Mode : "value") | undefined | ||
| }, | ||
| ): "promise" extends Mode ? ( | ||
| (value: W) => Promise<Result.Result.Success<R>> | ||
| ) : | ||
| "promiseExit" extends Mode ? ( | ||
| (value: W) => Promise<Exit.Exit<Result.Result.Success<R>, Result.Result.Failure<R>>> | ||
| ) : | ||
| ((value: W | ((value: R) => W)) => void) | ||
| { | ||
| if (options?.mode === "promise" || options?.mode === "promiseExit") { | ||
| return ((value: W) => { | ||
| registry.set(atom, value) | ||
| const promise = Effect.runPromiseExit( | ||
| AtomRegistry.getResult(registry, atom as Atom.Atom<Result.Result<any, any>>, { | ||
| suspendOnWaiting: true, | ||
| }), | ||
| ) | ||
| return options!.mode === "promise" ? promise.then(flattenExit) : promise | ||
| }) as any | ||
| } | ||
| return ((value: W | ((value: R) => W)) => { | ||
| registry.set(atom, typeof value === "function" ? (value as any)(registry.get(atom)) : value) | ||
| }) as any | ||
| } | ||
|
|
||
| const flattenExit = <A, E>(exit: Exit.Exit<A, E>): A => { | ||
| if (Exit.isSuccess(exit)) return exit.value | ||
| throw Cause.squash(exit.cause) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Multiple as any casts indicate type safety gaps.
The setAtom helper uses as any casts on lines 85, 88, and 89 to satisfy the complex conditional return type. While this works at runtime, it bypasses compile-time type checking of the return values.
Consider whether the complex mode-based overload typing is worth the loss of internal type safety, or refactor into separate functions:
function setAtomValue<R, W>(registry: AtomRegistry.Registry, atom: Atom.Writable<R, W>): (value: W | ((v: R) => W)) => void
function setAtomPromise<R, W>(registry: AtomRegistry.Registry, atom: Atom.Writable<Result.Result<any, any>, W>): (value: W) => Promise<...>Additionally, flattenExit (lines 92-95) throws on failure. Consider renaming to unwrapExitOrThrow or getOrThrow to make the throwing behavior explicit.
🤖 Prompt for AI Agents
In `@packages/solid/effect-atom/src/hooks.ts` around lines 62 - 95, The setAtom
function currently uses multiple unsafe casts ("as any") to satisfy its
conditional return type; refactor by splitting the implementation into explicit,
type-safe variants (e.g., setAtomValue, setAtomPromise, setAtomPromiseExit) or
by providing properly typed overloads for setAtom so the runtime branches
returned from the registry.set/Effect.runPromiseExit + AtomRegistry.getResult
paths have concrete types instead of any; also rename flattenExit to a
throwing-explicit name such as unwrapExitOrThrow (and update its signature and
call sites including where Effect.runPromiseExit is .then(flattenExit)) so the
throwing behavior is clear.
| /** | ||
| * @since 0.3.0 | ||
| * @category hooks | ||
| */ | ||
| export const useAtomRef = <A>(ref: AtomRef.ReadonlyRef<A>): Accessor<A> => { | ||
| const [value, setValue] = createSignal(ref.value) | ||
| onCleanup(ref.subscribe(setValue)) | ||
| return value | ||
| } | ||
|
|
||
| /** | ||
| * @since 0.3.0 | ||
| * @category hooks | ||
| */ | ||
| export const useAtomRefProp = <A, K extends keyof A>(ref: AtomRef.AtomRef<A>, prop: K): AtomRef.AtomRef<A[K]> => | ||
| ref.prop(prop) | ||
|
|
||
| /** | ||
| * @since 0.3.0 | ||
| * @category hooks | ||
| */ | ||
| export const useAtomRefPropValue = <A, K extends keyof A>(ref: AtomRef.AtomRef<A>, prop: K): Accessor<A[K]> => | ||
| useAtomRef(useAtomRefProp(ref, prop)) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
useAtomRefProp doesn't follow hook semantics.
useAtomRefProp (lines 206-207) is simply a passthrough to ref.prop(prop) with no Solid.js primitives (signals, effects, cleanup). The use prefix conventionally implies reactive behavior or lifecycle management. Consider either:
- Renaming to
getAtomRefPropor exporting it without the hook naming convention, or - Adding actual hook behavior if intended (e.g., memoization via
createMemo)
The composition in useAtomRefPropValue works correctly in Solid.js since component bodies run once.
♻️ Suggested rename for clarity
-export const useAtomRefProp = <A, K extends keyof A>(ref: AtomRef.AtomRef<A>, prop: K): AtomRef.AtomRef<A[K]> =>
+export const getAtomRefProp = <A, K extends keyof A>(ref: AtomRef.AtomRef<A>, prop: K): AtomRef.AtomRef<A[K]> =>
ref.prop(prop)
export const useAtomRefPropValue = <A, K extends keyof A>(ref: AtomRef.AtomRef<A>, prop: K): Accessor<A[K]> =>
- useAtomRef(useAtomRefProp(ref, prop))
+ useAtomRef(getAtomRefProp(ref, prop))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @since 0.3.0 | |
| * @category hooks | |
| */ | |
| export const useAtomRef = <A>(ref: AtomRef.ReadonlyRef<A>): Accessor<A> => { | |
| const [value, setValue] = createSignal(ref.value) | |
| onCleanup(ref.subscribe(setValue)) | |
| return value | |
| } | |
| /** | |
| * @since 0.3.0 | |
| * @category hooks | |
| */ | |
| export const useAtomRefProp = <A, K extends keyof A>(ref: AtomRef.AtomRef<A>, prop: K): AtomRef.AtomRef<A[K]> => | |
| ref.prop(prop) | |
| /** | |
| * @since 0.3.0 | |
| * @category hooks | |
| */ | |
| export const useAtomRefPropValue = <A, K extends keyof A>(ref: AtomRef.AtomRef<A>, prop: K): Accessor<A[K]> => | |
| useAtomRef(useAtomRefProp(ref, prop)) | |
| /** | |
| * `@since` 0.3.0 | |
| * `@category` hooks | |
| */ | |
| export const useAtomRef = <A>(ref: AtomRef.ReadonlyRef<A>): Accessor<A> => { | |
| const [value, setValue] = createSignal(ref.value) | |
| onCleanup(ref.subscribe(setValue)) | |
| return value | |
| } | |
| /** | |
| * `@since` 0.3.0 | |
| * `@category` hooks | |
| */ | |
| export const getAtomRefProp = <A, K extends keyof A>(ref: AtomRef.AtomRef<A>, prop: K): AtomRef.AtomRef<A[K]> => | |
| ref.prop(prop) | |
| /** | |
| * `@since` 0.3.0 | |
| * `@category` hooks | |
| */ | |
| export const useAtomRefPropValue = <A, K extends keyof A>(ref: AtomRef.AtomRef<A>, prop: K): Accessor<A[K]> => | |
| useAtomRef(getAtomRefProp(ref, prop)) |
🤖 Prompt for AI Agents
In `@packages/solid/effect-atom/src/hooks.ts` around lines 192 - 214,
useAtomRefProp is named like a Solid hook but merely returns ref.prop(prop)
without using Solid primitives; either rename useAtomRefProp to getAtomRefProp
(or remove the "use" prefix) wherever it's exported/used, or implement hook
semantics by wrapping the returned AtomRef in a Solid primitive (e.g.,
createMemo or createSignal) so it follows lifecycle/reactivity expectations;
update the implementation referenced by useAtomRefProp and ensure
useAtomRefPropValue still composes correctly with the renamed/adjusted function
(useAtomRefProp, useAtomRefPropValue, and ref.prop are the symbols to update).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/react-router-example/tsconfig.app.json`:
- Around line 12-13: Update the tsconfig alias and project settings: change the
`@prisma/`* paths mapping so it only points to the prisma/generated/effect/* (to
match the include "prisma/generated/effect/**/*") instead of the broader
"./prisma/generated/*", and verify that when "composite": true is set (in this
config or an inherited base) you also have "declaration": true enabled to avoid
TypeScript project-reference errors; adjust these options in tsconfig.app.json
accordingly.
| ".react-router/types/**/*", | ||
| "prisma/generated/effect/**/*" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what subdirectories exist under prisma/generated
echo "=== Subdirectories under prisma/generated ==="
if [ -d "prisma/generated" ]; then
find prisma/generated -maxdepth 1 -type d | sort
else
echo "prisma/generated not found at root"
fi
# Also check under apps/react-router-example
echo ""
echo "=== Subdirectories under apps/react-router-example/prisma/generated ==="
if [ -d "apps/react-router-example/prisma/generated" ]; then
find apps/react-router-example/prisma/generated -maxdepth 1 -type d | sort
else
echo "prisma/generated not found in app directory"
fi
# Check the full structure of the tsconfig.app.json
echo ""
echo "=== Full apps/react-router-example/tsconfig.app.json ==="
cat apps/react-router-example/tsconfig.app.json
# Check the base tsconfig.json
echo ""
echo "=== Full apps/react-router-example/tsconfig.json ==="
cat apps/react-router-example/tsconfig.jsonRepository: devx-op/effectify
Length of output: 1777
🏁 Script executed:
#!/bin/bash
# Check the root tsconfig.base.json
echo "=== Root tsconfig.base.json ==="
cat tsconfig.base.json 2>/dev/null || echo "File not found"Repository: devx-op/effectify
Length of output: 1045
Path alias scope should be narrowed to match includes.
The path alias @prisma/* maps to all of ./prisma/generated/*, but only prisma/generated/effect/**/* is included for type checking. Although the prisma/generated directory doesn't currently exist, if other subdirectories are created under it in the future, imports via @prisma/* to those paths would resolve at runtime but may not be type-checked.
Narrow the path alias to match the actual include:
💡 Suggested adjustment
"paths": {
- "@prisma/*": [
- "./prisma/generated/*"
+ "@prisma/effect/*": [
+ "./prisma/generated/effect/*"
]
}Additionally, composite: true typically requires declaration: true to be set. Verify this is configured (either here or in an inherited config) to avoid TypeScript project reference errors.
🤖 Prompt for AI Agents
In `@apps/react-router-example/tsconfig.app.json` around lines 12 - 13, Update the
tsconfig alias and project settings: change the `@prisma/`* paths mapping so it
only points to the prisma/generated/effect/* (to match the include
"prisma/generated/effect/**/*") instead of the broader "./prisma/generated/*",
and verify that when "composite": true is set (in this config or an inherited
base) you also have "declaration": true enabled to avoid TypeScript
project-reference errors; adjust these options in tsconfig.app.json accordingly.
* feat(solid-effect-atom)!: rewrite for @effect-atom/atom and Effect v3 (v0.3.0) BREAKING CHANGE: Complete rewrite of the library to wrap @effect-atom/atom. Adapts the API to be compatible with Effect v3 and provides SolidJS bindings similar to effect-smol. * fix: resolve typecheck errors in solid-example and update documentation links * fix(react-router-example): resolve build and tsconfig errors * fix(react-router-example): include generated types in tsconfig.app.json * fix(react-router-example): resolve typecheck output errors by refining includes * fix(react-router-example): resolve build vs typecheck conflict by setting outDir
1 participant
Complete rewrite of the library to wrap @effect-atom/atom. Adapts the API to be compatible with Effect v3. Includes full documentation and examples.
Summary by CodeRabbit
New Features
Breaking Changes
Documentation
Chores