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

fix(react-compiler): JSXText emits incorrect with bracket #32138

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

himself65
Copy link
Contributor

@himself65 himself65 commented Jan 20, 2025

Summary

Our LlamaIndex Product is blocked by this bug

Fixes: #32137

How did you test this change?

@himself65 himself65 force-pushed the himself65/2025/01/20/fix branch from 472c0cc to ee65f6c Compare January 20, 2025 23:36
@himself65 himself65 changed the title fix(react-compiler): JSXText emits incorrect with bracket fix(react-compiler): JSXText emits incorrect with bracket Jan 20, 2025
@mofeiZ
Copy link
Contributor

mofeiZ commented Jan 22, 2025

Thanks for the report and fix! Instead of adding a playground test, can you add this as a unit test fixture?

  1. Add the following in the fixtures directory

descriptive-name.jsx|tsx

// component to compile
function Test({friends}) {
  return (
    <div>
      If the string contains the string &#123;pageNumber&#125; it will be
      replaced by the page number.
    </div>
  );
}

// Export for test evaluator to know which function to invoke.
// Test evaluator will compare the result of compiledFn() against originalFn()
export const FIXTURE_ENTRYPOINT = {
  fn: Test,
  params: [{friends: []}],
};
  1. Run our test evaluator to update unit test snapshot files. (--watch or --filter mode might also be helpful)
# in react/compiler
$ yarn; yarn snap --update

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.

Looks good overall, please address feedback and we'll merge this 🙂

@himself65 himself65 force-pushed the himself65/2025/01/20/fix branch 3 times, most recently from ae79d12 to 6efadb9 Compare January 22, 2025 01:39
@himself65
Copy link
Contributor Author

Looks good overall, please address feedback and we'll merge this 🙂

Done

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.

Let's remove the changes to page.spec.ts (the playground test) and rename descriptive-name.jsx to a descriptive name, e.g. your PR title jsx-bracket-repro.jsx

@himself65 himself65 requested a review from mofeiZ January 22, 2025 06:57
@himself65 himself65 force-pushed the himself65/2025/01/20/fix branch from 6efadb9 to 86c00df Compare January 22, 2025 06:57
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.

Awesome, thanks for the fix!

@mofeiZ mofeiZ merged commit e5a2062 into facebook:main Jan 22, 2025
20 checks passed
@himself65 himself65 deleted the himself65/2025/01/20/fix branch January 23, 2025 23:37
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.

[Compiler Bug]: double quote compile incorrectly
3 participants