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

Extra parens inserted around JSX after attributes are added #1248

Open
michaldudak opened this issue Dec 12, 2022 · 14 comments
Open

Extra parens inserted around JSX after attributes are added #1248

michaldudak opened this issue Dec 12, 2022 · 14 comments

Comments

@michaldudak
Copy link

(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
Copy link

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

@eventualbuddha
Copy link
Collaborator

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
Copy link
Author

Michał did you find a workaround in the meantime?

We haven't upgraded to the broken version.

@JosephBrooksbank
Copy link

JosephBrooksbank commented Dec 28, 2022

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
Copy link
Author

@JosephBrooksbank
Copy link

That worked, thanks!

@dreyks
Copy link

dreyks commented Mar 2, 2023

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
Copy link

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

@dtyyy
Copy link

dtyyy commented Mar 3, 2023

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

@phitattoo
Copy link

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

coderaiser added a commit to coderaiser/putout that referenced this issue Mar 3, 2023
coderaiser added a commit to coderaiser/putout that referenced this issue Mar 3, 2023
@petergok
Copy link

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

@colinking
Copy link

colinking commented Jun 2, 2023

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
Copy link

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

@tim-dev
Copy link

tim-dev commented Jun 23, 2024

I believe this is fixed with #1406

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

No branches or pull requests