ts-migrate icon indicating copy to clipboard operation
ts-migrate copied to clipboard

ts-migrate is inserting comments in JSX components

Open flupke opened this issue 3 years ago • 17 comments

I'm giving ts-migrate a try to migrate a large gatsby project. My issue is JS comments are inserted in components:

export function Input(props: any) {
  return props.readonly ? (
    <div className={style.readonlyInputField} style={props.style}>
      {props.value}
    </div>
  ) : (
    <div className={style.inputField} style={props.style}>
      // @ts-expect-error ts-migrate(2607) FIXME: JSX element class does not support attributes beca... Remove this comment to see the full error message
      <RawInput className={style.input} {...props} />
    </div>
  )
}

This makes them visible in the rendered output. They should be inserted with braces instead:

export function Input(props: any) {
  return props.readonly ? (
    <div className={style.readonlyInputField} style={props.style}>
      {props.value}
    </div>
  ) : (
    <div className={style.inputField} style={props.style}>
      {/* @ts-expect-error ts-migrate(2607) FIXME: JSX element class does not support attributes beca... Remove this comment to see the full error message */}
      <RawInput className={style.input} {...props} />
    </div>
  )
}

I ran npx -p ts-migrate -c "ts-migrate-full ." at the root of the project. During the migration I saw a lot of occurrences of this error:

Error: [react-props][src/components/multiStates.tsx] Error:
 TypeError: Cannot read property 'length' of undefined
    at isReactSfcFunctionDeclaration (/home/flupke/src/jitter/jitter.video/frontend/node_modules/ts-migrate-plugins/build/src/plugins/utils/react.js:32:40)
    at Array.filter (<anonymous>)
    at Object.getNumComponentsInSourceFile (/home/flupke/src/jitter/jitter.video/frontend/node_modules/ts-migrate-plugins/build/src/plugins/utils/react.js:76:10)
    at Object.createPropsTypeNameGetter (/home/flupke/src/jitter/jitter.video/frontend/node_modules/ts-migrate-plugins/build/src/plugins/utils/react-props.js:239:41)
    at Object.run (/home/flupke/src/jitter/jitter.video/frontend/node_modules/ts-migrate-plugins/build/src/plugins/react-props.js:42:48)
    at Object.migrate (/home/flupke/src/jitter/jitter.video/frontend/node_modules/ts-migrate-server/build/src/migrate/index.js:65:46)
    at async Object.handler (/home/flupke/src/jitter/jitter.video/frontend/node_modules/ts-migrate/build/cli.js:130:22)

I have 108 .tsx files in the project so this will be cumbersome to fix, did I do something wrong during the conversion?

flupke avatar Nov 03 '21 17:11 flupke

Looking at ts-migrate code, the error comes from here, in ts-migrate-plugins/build/src/plugins/utils/react.js:

function isReactSfcFunctionDeclaration(functionDeclaration) {
    return (functionDeclaration.name != null &&
        /^[A-Z]\w*$/.test(functionDeclaration.name.text) &&
        functionDeclaration.parameters.length <= 2);  // <--------------
}

I tried handling the functionDeclaration.parameters === undefined case and the other similar errors I had, but this gave the same result as in the original issue.

flupke avatar Nov 04 '21 10:11 flupke

I'm experiencing the same problem. I tried to pinpoint the problem by stepping through with the debugger, but I was unable to find the point of failure.

I changed versions of TypeScript to see if that could be the issue, and found the issue does not exist when using v4.2.4. With this version, I get the expected output after running yarn ts-migrate migrate:

export default ({
  name
}: any) => {
  return (
    // @ts-expect-error ts-migrate(7026) FIXME: JSX element implicitly has type 'any' because no i... Remove this comment to see the full error message
    <div>
      {/* @ts-expect-error ts-migrate(7026) FIXME: JSX element implicitly has type 'any' because no i... Remove this comment to see the full error message */}
      <div>Hello, {name}!</div>
    {/* @ts-expect-error ts-migrate(7026) FIXME: JSX element implicitly has type 'any' because no i... Remove this comment to see the full error message */}
    </div>
  )
}

With versions 4.3.2 and 4.4.4, I get the following:

export default ({
  name
}: any) => {
  return (
    // @ts-expect-error ts-migrate(7026) FIXME: JSX element implicitly has type 'any' because no i... Remove this comment to see the full error message
    <div>
      // @ts-expect-error ts-migrate(7026) FIXME: JSX element implicitly has type 'any' because no i... Remove this comment to see the full error message
      <div>Hello, {name}!</div>
    // @ts-expect-error ts-migrate(7026) FIXME: JSX element implicitly has type 'any' because no i... Remove this comment to see the full error message
    </div>
  )
}

lnrdgmz avatar Nov 06 '21 01:11 lnrdgmz

Good detective work thank you! I'm sure this will help others a great deal, editing the hundreds of comments by hand was not fun at all :]

flupke avatar Nov 09 '21 09:11 flupke

@Rudeg @brieb @lencioni +1 i'm also able to repro this bug in [email protected] A meta question: can we add a help-wanted label? so that the community can pick those up and contribute these fixes?

sharmilajesupaul avatar Feb 16 '22 20:02 sharmilajesupaul

@Rudeg This is still reproducing on my project, I've attempted to change my project typescript version to 4.2.4, but it still happens. Any idea if #170 requires something extra to work?

matanwerbner avatar Aug 17 '22 10:08 matanwerbner

Also seeing exactly these issues: normal javascript // comments inserted inside TSX.

Tried rolling back to v0.1.28 which included TypeScript 4.2.4 but with no luck.

melv-n avatar Aug 31 '22 08:08 melv-n

I am noticing that this only happens on the last comment of each file. I have a number jsx files that generate +20 @ex-expect-error comments but only last comment uses // instead of the curly brackets.

typescript 4.8.4

Edit:

Its seems to be adding // @ts-expect-error TS(7026): JSX element implicitly has type 'any' because no i... Remove this comment to see the full error message before the last closing tag.

Calidus avatar Nov 07 '22 13:11 Calidus

I think there might be two related issues here:

  • why are we getting @ts-expect-error on closing tags? That just seems wrong to me if we have one on the opening tag.
  • ts-ignore doesn't properly handle adding comments to closing tags in tsx files. ts-ignore.inJsxText is returning false for closing tags.

Calidus avatar Nov 07 '22 15:11 Calidus

With the release of 0.1.35 I am not seeing any more incorrectly formatted comments in TSX files for the large project I am working on. I am still getting what I think are superfluous ts-expect-errors but that probably needs to be address with a new issue.

Calidus avatar Nov 08 '22 13:11 Calidus

i am still experiencing this on 0.1.35. i'm using typescript 4.8.4 and react 18.2.0.

could it be platform-dependent? i'm on linux, and my colleague on mac doesn't experience this issue.

szamanr avatar Nov 23 '22 10:11 szamanr

i am still experiencing this on 0.1.35. i'm using typescript 4.8.4 and react 18.2.0.

could it be platform-dependent? i'm on linux, and my colleague on mac doesn't experience this issue.

I was using Ubuntu via WSL with typscript 4.8.4. Are you seeing the miss formatted comments right before closing tags or in other locations?

Calidus avatar Nov 28 '22 13:11 Calidus

i am still experiencing this on 0.1.35. i'm using typescript 4.8.4 and react 18.2.0. could it be platform-dependent? i'm on linux, and my colleague on mac doesn't experience this issue.

I was using Ubuntu via WSL with typscript 4.8.4. Are you seeing the miss formatted comments right before closing tags or in other locations?

here's a screenshot. it's the git diff view: grey = before running script, green = after running script.

ts-migrate broke

szamanr avatar Nov 28 '22 13:11 szamanr

You may want to see verify the package lock file between you and your coworker to make sure you both using the exact same versions. I did a quick test of isJsxText using function foo { return ( HTML HERE ); } and it seems to be fine. I would recommend coming up with a minimal repo.

Calidus avatar Nov 28 '22 14:11 Calidus

For me, It worked with v4.7.2 which is the same version defined in package.json of ts-migrate repo.

noorulh06 avatar Dec 12 '22 03:12 noorulh06

This is broken with typescript 4.9.5, but not when I downgrade to 4.7.2.

jcurtis avatar Sep 25 '23 18:09 jcurtis

I'm getting comments inside the (t|j)sx expressions with TypeScript 5.0.4 Although I was able to get around this my temporarily downgrading to 4.7.2 and upgrading back, with only about ~10 manual edits needed after.

Thanks @jcurtis

AlexaDeWit avatar Feb 26 '24 14:02 AlexaDeWit