-
Notifications
You must be signed in to change notification settings - Fork 709
Fix JSX indentation in JavaScript output #1792
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: jakebailey <[email protected]>
Co-authored-by: jakebailey <[email protected]>
Uncommented the code in printer.go that respects the EFStartOnNewLine emit flag. This flag is set by the JSX transformer when transforming JSX elements with multiple children, and causes the printer to emit each child on a new line with proper indentation. This fix brings tsgo's JSX output into alignment with TypeScript's reference implementation, reducing the number of .diff files from 4488 to 37, and properly formatting nested JSX as: React.createElement(Child, null, React.createElement(Child, null, React.createElement(Child, null))); instead of: React.createElement(Child, null, React.createElement(Child, null, React.createElement(Child, null))); Co-authored-by: jakebailey <[email protected]>
...ommentsOnJSXExpressionsArePreserved(jsx=react,module=commonjs,moduledetection=force).js.diff
Outdated
Show resolved
Hide resolved
The previous fix incorrectly cleared the inJsxChild flag when transforming nested JSX children. When transforming a nested child element, the defer statement would set inJsxChild back to false, causing the parent element to not receive the EFStartOnNewLine flag. Fixed by saving and restoring the previous value of inJsxChild instead of unconditionally setting it to false. This ensures that the flag state is properly maintained across nested transformations. Also fixed the logic for adding EFStartOnNewLine to children: the flag should only be added when there are multiple children AFTER filtering out null/whitespace children, not before. This matches TypeScript's behavior where a single child expression does not get a newline, but multiple children do. These fixes resolve the regression in commentsOnJSXExpressionsArePreserved tests, where a single JSX expression child was incorrectly getting a newline. Co-authored-by: jakebailey <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes JSX indentation in JavaScript output to match TypeScript's behavior. The issue was that nested JSX elements were being collapsed to a single line instead of preserving proper indentation and newlines.
- Fixed JSX indentation by enabling the
EFStartOnNewLine
flag handling in the printer - Improved state management for JSX child transformations to prevent context corruption
- Updated newline logic to check children count after filtering out null children
Reviewed Changes
Copilot reviewed 91 out of 91 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
testdata/tests/cases/compiler/jsxNestedIndentation.tsx |
New test case to verify JSX nested indentation behavior |
testdata/tests/cases/compiler/jsxNestedIndentation.js |
Expected JavaScript output with proper indentation |
internal/transformers/jsxtransforms/jsx.go |
Fixed JSX child state management and newline logic for proper indentation |
internal/printer/printer.go |
Enabled EFStartOnNewLine flag handling to support multiline JSX output |
Various baseline files | Updated test baselines showing the fix applied across many JSX tests |
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.