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

printer: Support \n\n while printing JSXElement children #1396

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JHilker
Copy link

@JHilker JHilker commented Apr 1, 2024

The printer seems to treat any number of newlines within a JSXText as a single newline. This causes any transform touching these elements to remove newlines that tooling such as Prettier would otherwise not remove.

Though I'm a little surprised to not see the same formatting difference in ASTExplorer, though it's running on 0.21.1 instead of the latest version (0.23.6), so maybe this was a regression at some point? I can reproduce it both within the test suite, and transforms using 0.23.6.

Edit: Oh, this was because by default the transform itself was using parsers.esprima. It's reproducible if you swap to parsers.babel, example.

Current behavior:

Before After
const SomeComponent = () => {
  return (
    <div>
      <Foo />

      <Bar />
    </div>
  );
}
const SomeComponent = () => {
  return (
    <div>
      <FooMutated />
      <Bar />
    </div>
  );
}

This PR updates the printer for JSXElement / JSXFragment to return two newlines instead of a single newline for any string children which include two adjacent newlines.

Proposed behavior:

Before After
const SomeComponent = () => {
  return (
    <div>
      <Foo />

      <Bar />
    </div>
  );
}
const SomeComponent = () => {
  return (
    <div>
      <FooMutated />

      <Bar />
    </div>
  );
}

Thanks in advance for any review. This seems like an innocent enough change, but I'd entirely understand if it's more complicated than I'm imagining. Open to feedback and happy to iterate on this PR!

@@ -1315,6 +1315,8 @@ function genericPrintNoParens(path: any, options: any, print: any) {
) {
if (/\S/.test(child.value)) {
return child.value.replace(/^\s+|\s+$/g, "");
} else if (/\n\n/.test(child.value)) {
Copy link

Choose a reason for hiding this comment

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

Fairly pedantic, but this won't work if there is any whitespace on the second empty line. You might consider making the regex something like /\n[ \t]*\n/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants