Skip to content
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

[compiler] Allow all hooks to take callbacks which access refs, but ban hooks from taking direct ref value arguments #30917

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

mvitousek
Copy link
Contributor

@mvitousek mvitousek commented Sep 8, 2024

Stack from ghstack (oldest at bottom):

Summary:
This brings the behavior of ref mutation within hook callbacks into alignment with the behavior of global mutations--that is, we allow all hooks to take callbacks that may mutate a ref. This is potentially unsafe if the hook eagerly calls its callback, but the alternative is excessively limiting (and inconsistent with other enforcement).

This also bans directly passing a ref.current value to a hook, which was previously allowed.

…an hooks from taking direct ref value arguments

Summary:
This brings the behavior of ref mutation within hook callbacks into alignment with the behavior of global mutations--that is, we allow all hooks to take callbacks that may mutate a ref. This is potentially unsafe if the hook eagerly calls its callback, but the alternative is excessively limiting (and inconsistent with other enforcement).

This also bans *directly* passing a ref.current value to a hook, which was previously allowed.

[ghstack-poisoned]
Copy link

vercel bot commented Sep 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 16, 2024 5:58pm

mvitousek added a commit that referenced this pull request Sep 8, 2024
…an hooks from taking direct ref value arguments

Summary:
This brings the behavior of ref mutation within hook callbacks into alignment with the behavior of global mutations--that is, we allow all hooks to take callbacks that may mutate a ref. This is potentially unsafe if the hook eagerly calls its callback, but the alternative is excessively limiting (and inconsistent with other enforcement).

This also bans *directly* passing a ref.current value to a hook, which was previously allowed.

ghstack-source-id: 11024355a4e6b8cb2464da7819b52e505d327299
Pull Request resolved: #30917
@@ -362,6 +362,17 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
returnValueKind: ValueKind.Mutable,
}),
],
[
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is left over from a previous version of this work, but seems reasonable to add anyways

```

### Eval output
(kind: exception) useRef is not defined
Copy link
Contributor

Choose a reason for hiding this comment

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

oops

Copy link
Contributor

@josephsavona josephsavona left a comment

Choose a reason for hiding this comment

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

awesome

…refs, but ban hooks from taking direct ref value arguments"

Summary:
This brings the behavior of ref mutation within hook callbacks into alignment with the behavior of global mutations--that is, we allow all hooks to take callbacks that may mutate a ref. This is potentially unsafe if the hook eagerly calls its callback, but the alternative is excessively limiting (and inconsistent with other enforcement).

This also bans *directly* passing a ref.current value to a hook, which was previously allowed.

[ghstack-poisoned]
mvitousek added a commit that referenced this pull request Sep 16, 2024
…an hooks from taking direct ref value arguments

Summary:
This brings the behavior of ref mutation within hook callbacks into alignment with the behavior of global mutations--that is, we allow all hooks to take callbacks that may mutate a ref. This is potentially unsafe if the hook eagerly calls its callback, but the alternative is excessively limiting (and inconsistent with other enforcement).

This also bans *directly* passing a ref.current value to a hook, which was previously allowed.

ghstack-source-id: e66ce7123ecf4a905adab957970d0ee5d41245e0
Pull Request resolved: #30917
@mvitousek mvitousek merged commit a53a513 into gh/mvitousek/29/base Sep 16, 2024
17 of 19 checks passed
@mvitousek mvitousek deleted the gh/mvitousek/29/head branch September 16, 2024 17:57
github-actions bot pushed a commit that referenced this pull request Sep 16, 2024
…an hooks from taking direct ref value arguments

Summary:
This brings the behavior of ref mutation within hook callbacks into alignment with the behavior of global mutations--that is, we allow all hooks to take callbacks that may mutate a ref. This is potentially unsafe if the hook eagerly calls its callback, but the alternative is excessively limiting (and inconsistent with other enforcement).

This also bans *directly* passing a ref.current value to a hook, which was previously allowed.

ghstack-source-id: e66ce7123ecf4a905adab957970d0ee5d41245e0
Pull Request resolved: #30917

DiffTrain build for [e78c936](e78c936)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants