Skip to content

InferEffectDeps takes a React.AUTODEPS sigil #33799

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

Jump to bottom
Merged
jbrown215 merged 1 commit into from
Jul 17, 2025
Merged

InferEffectDeps takes a React.AUTODEPS sigil #33799

jbrown215 merged 1 commit into from
Jul 17, 2025

Conversation

@jbrown215
Copy link
Contributor

@jbrown215 jbrown215 commented Jul 16, 2025 •
edited
Loading

@jbrown215 jbrown215 mentioned this pull request Jul 16, 2025
Merged
@meta-cla meta-cla bot added the CLA Signed label Jul 16, 2025
jbrown215
jbrown215 commented Jul 16, 2025
View reviewed changes
compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts
if (
value.args.length === autodepFnLoads.get(callee.identifier.id) &&
value.args.length > 1 &&
autodepsArgIndex > 0 &&
Copy link
Contributor Author

@jbrown215 jbrown215 Jul 16, 2025

Choose a reason for hiding this comment

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

In the next diff I change the config to specify the index for autodeps instead of number of required args

mofeiZ
mofeiZ reviewed Jul 16, 2025
View reviewed changes
compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts
BuiltInUseEffectEventId,
),
],
['AUTODEPS', addObject(DEFAULT_SHAPES, BuiltInAutodepsId, [])],
Copy link
Contributor

@mofeiZ mofeiZ Jul 16, 2025

Choose a reason for hiding this comment

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

supernit -- should this be prefixed with experimental_ / unstable_?

mofeiZ
mofeiZ reviewed Jul 16, 2025
View reviewed changes
...ler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-granular-access.expect.md
```javascript
// @inferEffectDependencies @panicThreshold:"none"
import { useEffect } from "react";
import { useEffect, AUTODEPS } from "react";
Copy link
Contributor

@mofeiZ mofeiZ Jul 16, 2025

Choose a reason for hiding this comment

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

Makes sense that we don't rewrite the import (yet)

mofeiZ
mofeiZ reviewed Jul 16, 2025
View reviewed changes
...c/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/no-emit-lint-repro.expect.md
function ReactiveVariable({ propVal }) {
const arr = [propVal];
useEffectWrapper(() => print(arr));
useEffectWrapper(() => print(arr), AUTODEPS);
Copy link
Contributor

@mofeiZ mofeiZ Jul 16, 2025

Choose a reason for hiding this comment

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

This makes sense since we're in no-emit

mofeiZ
mofeiZ approved these changes Jul 16, 2025
View reviewed changes
Copy link
Contributor

@mofeiZ mofeiZ left a comment

Choose a reason for hiding this comment

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

Nice!! I read through the fixtures to confirm that (1) we're currently detecting + deleting AUTODEPS references and (2) logger diagnostics stay unchanged (except for line numbers)

@jbrown215 jbrown215 merged commit dffacc7 into facebook:main Jul 17, 2025
28 checks passed
github-actions bot pushed a commit that referenced this pull request Jul 17, 2025
@jbrown215
InferEffectDeps takes a React.AUTODEPS sigil (#33799)
fef10fd 
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33799).
* #33800
* __->__ #33799

DiffTrain build for [dffacc7](dffacc7)
github-actions bot pushed a commit that referenced this pull request Jul 17, 2025
@jbrown215
InferEffectDeps takes a React.AUTODEPS sigil (#33799)
e55cc20 
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33799).
* #33800
* __->__ #33799

DiffTrain build for [dffacc7](dffacc7)
@nextjs-bot nextjs-bot mentioned this pull request Jul 17, 2025
Merged
github-actions bot pushed a commit to code/lib-react that referenced this pull request Jul 20, 2025
@pull
InferEffectDeps takes a React.AUTODEPS sigil (facebook#33799)
5cd660b 
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33799).
* facebook#33800
* __->__ facebook#33799

DiffTrain build for [dffacc7](facebook@dffacc7)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Jul 20, 2025
@pull
InferEffectDeps takes a React.AUTODEPS sigil (facebook#33799)
2b9463d 
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33799).
* facebook#33800
* __->__ facebook#33799

DiffTrain build for [dffacc7](facebook@dffacc7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mofeiZ mofeiZ mofeiZ approved these changes

Assignees

No one assigned

Labels

CLA Signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jbrown215 @mofeiZ