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] Rewrite useContext callee #30612

Merged
merged 6 commits into from
Aug 8, 2024
Merged

Conversation

gsathya
Copy link
Member

@gsathya gsathya commented Aug 6, 2024

Stack from ghstack (oldest at bottom):

If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

[ghstack-poisoned]
Copy link

vercel bot commented Aug 6, 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 Aug 7, 2024 3:01pm

gsathya added a commit that referenced this pull request Aug 6, 2024
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

ghstack-source-id: 60120858972dacc54bfc55daaadd0924a10f2cba
Pull Request resolved: #30612
@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Aug 6, 2024
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

[ghstack-poisoned]
gsathya added a commit that referenced this pull request Aug 6, 2024
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

ghstack-source-id: c305c4dffd4a9b794320ded734520ff35ade05db
Pull Request resolved: #30612
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

[ghstack-poisoned]
gsathya added a commit that referenced this pull request Aug 6, 2024
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

ghstack-source-id: c305c4dffd4a9b794320ded734520ff35ade05db
Pull Request resolved: #30612
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

[ghstack-poisoned]
gsathya added a commit that referenced this pull request Aug 6, 2024
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

ghstack-source-id: c305c4dffd4a9b794320ded734520ff35ade05db
Pull Request resolved: #30612
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

[ghstack-poisoned]
gsathya added a commit that referenced this pull request Aug 6, 2024
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

ghstack-source-id: 0a4a3b6bbbbd4f75591f7a99a35ca23e56f6ba08
Pull Request resolved: #30612
@@ -15,7 +15,8 @@ function App() {
## Code

```javascript
import { c as _c } from "react/compiler-runtime"; // @enableLowerContextAccess
import { useContext_withSelector } from "react-compiler-runtime";
import { c as _c } from "react/compiler-runtime"; // @lowerContextAccess
function App() {
const $ = _c(3);
const context = useContext(MyContext);
Copy link
Member

Choose a reason for hiding this comment

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

name doesn't match the name of the generated import

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah because it's not lowered here -- note the todo in the filename

Copy link
Member Author

Choose a reason for hiding this comment

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

i need to keep some state to check if we lower or not and propagate to the result, not sure if it's worth doing? is it an issue to have the unused imports?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jackpope said this might be an issue so fixed here #30628

@gsathya gsathya requested a review from kassens August 7, 2024 14:53
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

[ghstack-poisoned]
gsathya added a commit that referenced this pull request Aug 7, 2024
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

ghstack-source-id: 00e161b988c8f8a1cf96efff8095f050cb534cc1
Pull Request resolved: #30612
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! Looks good with the later changes to avoid imports unless they're used

@gsathya gsathya merged commit 5112793 into gh/gsathya/15/base Aug 8, 2024
19 checks passed
gsathya added a commit that referenced this pull request Aug 8, 2024
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

ghstack-source-id: 00e161b988c8f8a1cf96efff8095f050cb534cc1
Pull Request resolved: #30612
@gsathya gsathya deleted the gh/gsathya/15/head branch August 8, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants