fix(react-compiler): implement NumericLiteral as ObjectPropertyKey #31791
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hi @dimaMachina! Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours needs attention. You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
This comment was marked as spam.
This comment was marked as spam.
|
friendly ping @josephsavona as you requested to create PR with fix in twitter |
This comment was marked as spam.
This comment was marked as spam.
|
Yeah, will take a look tomorrow |
| 21: 'dimaMachina' | ||
| } | ||
| return <div>{obj[21]}</div> | ||
| } |
There was a problem hiding this comment.
please add a fixture here so that we can test evaluation with/without compilation:
export const FIXTURE_ENTRYPOINT = {
fn: Test,
params: [{}],
};
There was a problem hiding this comment.
also be sure to update fixtures after this
There was a problem hiding this comment.
Thank you! Updated! Could you also review my other contribution to the react compiler? #31792
I just pushed there similar change 🙂
|
Thanks for taking your time on my little efforts🙏🙏
…On Fri, 10 Jan, 2025, 22:07 Joseph Savona, ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Just one change, then this should be good to land. Thanks!
—
Reply to this email directly, view it on GitHub
<#31791 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A46NNG53CXXOP2TJDCEOQN32J7ZK7AVCNFSM6AAAAABTU3XIUCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNBSHEZDONRRGI>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
@josephsavona friendly ping 🙏 |
|
Thanks for the contribution! Please run |
|
@mofeiZ ty, done! |
|
@mofeiZ friendly ping |
| } else if (key.isNumericLiteral()) { | ||
| return { | ||
| kind: 'identifier', | ||
| name: String(key.node.value), | ||
| }; |
There was a problem hiding this comment.
Let's also avoid converting numeric literals to babel identifiers. Could you
- Add a 'number' variant to HIR:ObjectPropertyKey
- Check for the number variant in codegenReactiveFunction:codegenObjectPropertyKey and return a
t.numericLiteral
Let's also add a test case for destructuring assignment, which also uses ObjectExpressions
const {21: myVar} = obj;
There was a problem hiding this comment.
Awesome, thanks for the quick fix! Just started a test run and will merge once everything passes
Should I move destructuring assignment test to a separate file?
This should be fine.
There was a problem hiding this comment.
Great, updated snapshot in 1fb22ae
fix https://x.com/dimaMachina_/status/1867706100264652875 cc @josephsavona