dioxus icon indicating copy to clipboard operation
dioxus copied to clipboard

Lint Potential Recursive Hook Loops Errors

Open agreyyy opened this issue 2 years ago • 3 comments

Specific Demand

The hooks with dependencies should have a compile time check for the dependancy being changed inside the hook in order to not have recursive loops and system crashes in runtime. For example this code should produce a compile time error: `fn EditableHeader<'a>(cx:Scope, val: String, onedit: EventHandler<'a,Option<String>>, globally_editable: bool) -> Element {

let locally_editable = use_state(cx, || {globally_editable.to_owned()}); 

let _ = use_memo(cx,(globally_editable, locally_editable), |(globally_editable,_)| {
    locally_editable.set(globally_editable.to_owned()); //error: this hook changes the value of its own dependancy!!! 
});

//irrelevant code below `

Implement Suggestion

I do not know how to implement this as I am just learning how to use the framework and do not understand how it works. but i imagine you can probably hardcode any hooks with dependancys in them to produce an error if a UseState.set() or UseRef.with_mut() is used on any of the dependancies inside them. This is a simple user error and this feature probably shouldn`t be a high priority.

agreyyy avatar Sep 30 '23 11:09 agreyyy

This would be great to add to dioxus-check! We could hard code some hooks with dependancies initially. Once a more complete linting system that has more complete access to the source code like clippy, we could potentially detect this error for custom hooks as well.

We could also detect a similar issue that happens when you write to a hook within the main body of a component/hook:

let locally_editable = use_state(cx, || 0); 
// If you unconditionally write to locally_editable, it will cause an infinite loop
locally_editable.set(1);

ealmloff avatar Sep 30 '23 14:09 ealmloff

This would be great to add to dioxus-check! We could hard code some hooks with dependancies initially. Once a more complete linting system that has more complete access to the source code like clippy, we could potentially detect this error for custom hooks as well.

We could also detect a similar issue that happens when you write to a hook within the main body of a component/hook:

let locally_editable = use_state(cx, || 0); 
// If you unconditionally write to locally_editable, it will cause an infinite loop
locally_editable.set(1);

I think you could try and modify the visit_expr_call() function match statement to have an explicit Node::HookFn(_) arm. Then inside that arm you can conditionally check if a use_ref write() or with_mut() and/or use_state.set() is used and then return a new issue type called something like UnconditionalDependencyMutation or InfinitelyRecursiveHookError. Then in that issue`s display implementation you can put in the error message that you wrote above this comment. I just started reading the code that you linked to, so I might not be understanding how the error handling system works yet.

agreyyy avatar Oct 01 '23 10:10 agreyyy

With signals, we can warn at runtime if a signal writes to a reactive context that subscribes to it

ealmloff avatar Mar 12 '24 19:03 ealmloff