Skip to content

fix: using get with items with onChange is now correctly typed #577

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

fix: using get with items with onChange is now correctly typed #577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5511ee9
Initial plan
Copilot Nov 2, 2025
386a69e
Fix: Propagate IncludeTransient flag for folder items with onChange
Copilot Nov 2, 2025
004527b
Add clarifying comment for recursive Tree call in folders
Copilot Nov 2, 2025
058453c
Merge branch 'main' into copilot/fix-onchange-function-issue
gsimone Nov 8, 2025
43039df
changeset
gsimone Nov 8, 2025
96e1c1a
Merge branch 'main' into copilot/fix-onchange-function-issue
gsimone Nov 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/dirty-radios-laugh.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"leva": patch
---

fix: using get with items with onChange is now correctly typed
46 changes: 46 additions & 0 deletions packages/leva/src/types/public.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,52 @@
})
)

// folders with onChange should exclude transient items from return type but include them in get function
expectType<{ a1: number }>(
useControls({
a: folder({
a1: 1,
a2: { value: 'transient', onChange: () => {} },
}),
})
)

// test that get function includes transient items in folders
expectType<
[
{ a1: number },
(value: { a1?: number; a2?: string }) => void,
<T extends 'a1' | 'a2'>(path: T) => { a1: number; a2: string }[T]
]
>(
useControls(() => ({

Check failure on line 158 in packages/leva/src/types/public.test.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test 

Argument of type 'FunctionReturnType<{ a: FolderInput<{ a1: number; }>; }>' is not assignable to parameter of type '[{ a1: number; }, (value: { a1?: number | undefined; a2?: string | undefined; }) => void, <T extends "a1" | "a2">(path: T) => { a1: number; a2: string; }[T]]'.
a: folder({
a1: 1,
a2: { value: 'transient', onChange: () => {} },
}),
}))
)

// nested folders with onChange
expectType<
[
{ a1: number },
(value: { a1?: number; b1?: number; b2?: string }) => void,
<T extends 'a1' | 'b1' | 'b2'>(path: T) => { a1: number; b1: number; b2: string }[T]
]
>(
useControls(() => ({

Check failure on line 174 in packages/leva/src/types/public.test.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test 

Argument of type 'FunctionReturnType<{ a: FolderInput<{ b1: number; a1: number; }>; }>' is not assignable to parameter of type '[{ a1: number; }, (value: { a1?: number | undefined; b1?: number | undefined; b2?: string | undefined; }) => void, <T extends "a1" | "b1" | "b2">(path: T) => { a1: number; b1: number; b2: string; }[T]]'.
a: folder({
a1: 1,
a2: { value: 'transient', onChange: () => {} },
b: folder({
b1: { value: 10 },
b2: { value: 'also transient', onChange: () => {} },
}),
}),
}))
)
Comment on lines +150 to +184
Copy link

@coderabbitai coderabbitai bot Nov 8, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Type assertions currently fail – fix the folder typing first.

The new expectType blocks don’t compile right now because useControls(...)[2] still reports only 'a1' | 'b1' paths; 'a2'/'b2' are dropped by the existing types. Until the underlying Tree/folder typing is fixed, these tests will keep failing (yarn test:types hits the same FunctionReturnType error shown in CI). Please address the root cause in the type definitions and re-run the type tests.

After updating the types, re-check with:

🏁 Script executed:

#!/bin/bash
yarn test:types

Length of output: 747


---

🏁 Script executed:

```shell
#!/bin/bash
# Find TypeScript/JS files related to useControls and type definitions
fd -e ts -e tsx | grep -E "(useControls|types|folder)" | head -20

Length of output: 861

🏁 Script executed:

#!/bin/bash
# Look at the full test file to understand context
cat packages/leva/src/types/public.test.ts

Length of output: 6861

🏁 Script executed:

#!/bin/bash
# Search for useControls implementation
rg -n "export.*useControls" --type ts --type tsx -A 5

Length of output: 27

🏁 Script executed:

#!/bin/bash
# Search for folder function/type definition
rg -n "export.*folder" --type ts --type tsx -A 5 | head -50

Length of output: 27

🏁 Script executed:

#!/bin/bash
# Look at the type definitions
cat packages/leva/src/types/public.ts

Length of output: 10685

🏁 Script executed:

#!/bin/bash
# Look at internal types
cat packages/leva/src/types/internal.ts

Length of output: 3049

🏁 Script executed:

#!/bin/bash
# Look at folder.ts implementation
cat packages/leva/src/helpers/folder.ts

Length of output: 403

🏁 Script executed:

#!/bin/bash
# Search for Tree type definition
rg -n "type Tree" packages/leva/src/types/ -A 10

Length of output: 1878

🏁 Script executed:

#!/bin/bash
# Search for the get function type signature
rg -n "get\s*\(" packages/leva/src/types/public.ts -B 3 -A 3

Length of output: 168

🏁 Script executed:

#!/bin/bash
# Look at useControls implementation for type signatures
cat packages/leva/src/useControls.ts

Length of output: 8864

🏁 Script executed:

#!/bin/bash
# Look at headless useControls
cat packages/leva/src/headless/useControls.ts

Length of output: 2244

🏁 Script executed:

#!/bin/bash
# Search for useControls type signature in type files
rg -n "useControls" packages/leva/src/types/ -B 2 -A 8

Length of output: 14199

Type assertions fail due to folder() filtering transient items too early.

The getter function type correctly expects transient items (line 16 of useControls.ts uses SchemaToValues<S, true>), but folder() (folder.ts) returns FolderInput<SchemaToValues<S>>, which prematurely filters out transient items. When Tree later tries to process the folder schema with IncludeTransient=true, those items are already gone from the type.

Fix: Change folder.ts line 4 from FolderInput<SchemaToValues<S>> to FolderInput<S> so the raw schema reaches Tree, which can then apply the IncludeTransient flag correctly.

🧰 Tools
🪛 Biome (2.1.2)

[error] 158-158: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level

(lint/correctness/useHookAtTopLevel)

[error] 174-174: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level

(lint/correctness/useHookAtTopLevel)

🪛 GitHub Check: Build, lint, and test

[failure] 174-174:
Argument of type 'FunctionReturnType<{ a: FolderInput<{ b1: number; a1: number; }>; }>' is not assignable to parameter of type '[{ a1: number; }, (value: { a1?: number | undefined; b1?: number | undefined; b2?: string | undefined; }) => void, <T extends "a1" | "b1" | "b2">(path: T) => { a1: number; b1: number; b2: string; }[T]]'.

[failure] 158-158:
Argument of type 'FunctionReturnType<{ a: FolderInput<{ a1: number; }>; }>' is not assignable to parameter of type '[{ a1: number; }, (value: { a1?: number | undefined; a2?: string | undefined; }) => void, <T extends "a1" | "a2">(path: T) => { a1: number; a2: string; }[T]]'.

🤖 Prompt for AI Agents 
In packages/leva/src/folder.ts around line 4, folder() currently types its
argument as FolderInput<SchemaToValues<S>>, which strips transient items too
early; change the signature to FolderInput<S> so the raw schema (including
transient markers) is passed through to Tree and the IncludeTransient flag can
be applied later.


/**
* custom plugins
*/
Expand Down
6 changes: 5 additions & 1 deletion packages/leva/src/types/public.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,11 @@ type Join<Leaf1, Leaf1Key extends keyof Leaf1, Leaf2> = EndLeaf extends Leaf2

type Tree<IncludeTransient extends boolean, Leaf, LeafKey extends string | number | symbol = ''> = {
// if it's a folder we run the type check on it's schema key
0: Leaf extends { schema: infer Schema } ? { [Key in keyof Schema]: Join<Schema, Key, Schema[Key]> } : never
// recursively process each item in the folder with Tree to propagate IncludeTransient flag,
// which ensures items with onChange inside folders are correctly handled for the get function
0: Leaf extends { schema: infer Schema }
? { [Key in keyof Schema]: Join<Schema, Key, Tree<IncludeTransient, Schema[Key], Key>> }[keyof Schema]
: never
Comment on lines +244 to +248
Copy link

@coderabbitai coderabbitai bot Nov 8, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Folder recursion still hides transient keys from get() typing.

Even with this recursion, FolderInput’s schema generic has already stripped transient entries (e.g. it instantiates as { a1: number } in the new tests), so Schema[Key] never exposes 'a2' / 'b2'. As a result the Tree walk cannot reintroduce those keys and useControls(...)[2] still rejects get('a2'), exactly as CI reports (FunctionReturnType<{ a: FolderInput<{ a1: number; }>; }> not assignable …). The patch therefore doesn’t fix #447 and the added tests fail. Please propagate the original folder schema—e.g. carry both the value shape and the raw schema (including transients) through FolderInput—or otherwise ensure Tree<IncludeTransient, …> receives a type that still contains transient keys so the union can include them.

🤖 Prompt for AI Agents 
In packages/leva/src/types/public.ts around lines 244 to 248, the recursive Tree
walk is using the FolderInput's already-stripped schema so transient keys are
lost and get() typings can’t include them; update the FolderInput type (or the
place where folder schemas are passed into Tree) to carry both the processed
value shape and the original/raw schema (including transient entries) so
Tree<IncludeTransient, ...> receives the raw schema with transient keys intact;
change FolderInput generics to accept and preserve a second type parameter for
the raw schema (or embed a RawSchema field) and thread that raw schema into the
Tree recursion so the union can reintroduce transient keys for get().

1: never
// if the leaf is an object, we run the type check on each of its keys
2: {
Expand Down
Loading