-
Notifications
You must be signed in to change notification settings - Fork 48.3k
[compiler][autodeps/fire] Do not include fire functions in autodep arrays #32532
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
Conversation
…rays Summary: We landed on not including fire functions in dep arrays. They aren't needed because all values returned from the useFire hook call will read from the same ref. The linter will error if you include a fired function in an explicit dep array. Test Plan: yarn snap --watch --
isSetStateType(dep.identifier)) && | ||
!reactiveIds.has(dep.identifier.id) | ||
!reactiveIds.has(dep.identifier.id)) || | ||
isFireFunctionType(dep.identifier) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh makes sense, fire is different than setState / refs in that we always codegen the useFire
hook call, so there is no "reactive reference to non-reactive values" issue (e.g. props.cond ? ref1 : ref2
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, exactly!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
…rays (#32532) Summary: We landed on not including fire functions in dep arrays. They aren't needed because all values returned from the useFire hook call will read from the same ref. The linter will error if you include a fired function in an explicit dep array. Test Plan: yarn snap --watch -- DiffTrain build for [b8bedc2](b8bedc2)
Summary: We landed on not including fire functions in dep arrays. They aren't needed because all values returned from the useFire hook call will read from the same ref. The linter will error if you include a fired function in an explicit dep array.
Test Plan: yarn snap --watch
--