Extra parens inserted around JSX after attributes are added

[michaldudak]

(This was originally posted in jscodeshift's repo: facebook/jscodeshift#534, but I was redirected to recast)

Context
We're using jscodeshift (and, thus, recast) to provide codemods that help our users migrate to new MUI libraries versions. After updating jscodeshift to 0.14.0 (which updates recast to 0.21.0), our tests started to fail.
It seems that when a codemod adds a JSX attribute, the whole JSX block in a return statement gets wrapped in extra parens (even if it had parens before).

I managed to create a minimal repro:

index.js

const fs = require("fs");
const recast = require("recast");
const { builders: b, visit } = require("ast-types");

const source = fs.readFileSync("source.js", "utf8");

const ast = recast.parse(source, {
  parser: require("recast/parsers/babel"),
});

visit(ast, {
  visitJSXElement: function (path) {
    let attributes = path.node.openingElement.attributes;
    attributes.push(
      b.jsxAttribute(b.jsxIdentifier("data-foo"), b.literal("bar"))
    );

    this.traverse(path);
  },
});

console.log(recast.print(ast).code);

source.js

import * as React from "react";

export default function WrappedLink(props) {
  return (
    <div>
      <a {...props} />
    </div>
  );
}

expected output

import * as React from "react";

export default function WrappedLink(props) {
  return (
    <div data-foo="bar">
      <a {...props} data-foo="bar" />
    </div>
  );
}

actual output

import * as React from "react";

export default function WrappedLink(props) {
  return (
    (<div data-foo="bar">
      <a {...props} data-foo="bar" />
    </div>)
  );
}

[JosephBrooksbank]

Looks like a fix has been up and ready for review for 2 weeks. Michał did you find a workaround in the meantime?

[eventualbuddha]

Looks like a fix has been up and ready for review for 2 weeks. Michał did you find a workaround in the meantime?

#1257 isn't really a fix, is it? A proper fix would be to not add the double parens in the first place, right?

[michaldudak]

Michał did you find a workaround in the meantime?

We haven't upgraded to the broken version.

[JosephBrooksbank]

We haven't upgraded to the broken version.

Which version of jscodeshift and recast are you using?

#1257 isn't really a fix, is it? A proper fix would be to not add the double parens in the first place, right?

It is a workaround, which would be good enough for my use-case. It seems like there are a myriad of issues surrounding parentheses in recast right now, so any path forward is better than nothing imo

[michaldudak]

[email protected] and [email protected]

[JosephBrooksbank]

That worked, thanks!

[dreyks]

looks like it has something to do with the newlines and indentations

import * as React from "react";

export default function WrappedLink(props) {
  return (<div><a {...props} /></div>);
}

^this produces correct results

import * as React from "react";

export default function WrappedLink(props) {
  return (<div><a {...props} /></div>
  );
}

^ this as well

import * as React from "react";

export default function WrappedLink(props) {
  return (<div><a {...props} />
    </div>
  );
}

^ but this gets double parentheses

[michaelfaith]

We're affected by this issue as well. Thanks for logging it.

[dtyyy]

I also encountered it when i was writing an automation script,thanks!

[phitattoo]

I also encountered it when i was writing an automation script,thanks!

[petergok]

I've also encountered this error, +1 for a potential fix 🙏

[colinking]

Upgrading ast-types from the latest tag (0.14.2) to the next tag (0.16.1) appears to be a partial fix. Doing so resolves a similar issue that we ran into, but it doesn't appear to help with OP's example.

Specifically, we had code like so:

import { Callout, Heading, Link, Stack, Text } from "@airplane/views";
import airplane from "airplane";

const TestView = () => {
  return (
    <Stack spacing="lg">
      <Stack>
        <Text>Views make it easy to build UIs in Airplane.</Text>
      </Stack>
    </Stack>
  );
};

export default airplane.view(
  {
    slug: "test_view",
    name: "Test view"
  },
  TestView
);

When using ast-types=0.14.2 and calling parse -> visit -> print on this code (without even performing any changes to the AST), it would produce the following:

import { Callout, Heading, Link, Stack, Text } from "@airplane/views";
import airplane from "airplane";

const TestView = () => {
  return (
    (<Stack spacing="lg">
      <Stack>
              <Text>Views make it easy to build UIs in Airplane.</Text>
            </Stack>
    </Stack>)
  );
};

export default airplane.view(
  {
    slug: "test_view",
    name: "Test view"
  },
  TestView
);

[WalterWeidner]

+1 just spent way too much time tracking this down and trying to determine it wasn't something goofy with my configuration.

[tim-dev]

I believe this is fixed with #1406