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

Migrating explicit-any takes far longer than I expected #121

Open
Cactice opened this issue May 26, 2021 · 5 comments
Open

Migrating explicit-any takes far longer than I expected #121

Cactice opened this issue May 26, 2021 · 5 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@Cactice
Copy link

Cactice commented May 26, 2021

[explicit-any] Finished in 1h 32m 5.3s.
.
.
.
Finished in 1h 48m 32.9s, for 14 plugin(s).

At the end there were 47 files that were updated. Most of which were less than 200 lines.
lines = 200x14=2800
seconds to finish=92x60=5520
seconds per line = 5520/2280 = 2.42s

This is a very rough estimate but hopefully this information is helpful for anybody worried if there's something wrong with explicit-any.

Please note that it may take a long time to do a full migration.

This is helpful but given most of the other plugins finished in less than a few minutes,
I was expecting 10-20 min for explicit-any but this took way longer than that so I thought there was something wrong with the plugin.

@Cactice
Copy link
Author

Cactice commented May 26, 2021

I will close this.
I just wanted to leave a note for those who were worried if explicit-any is stuck or is just very slow.

@Cactice Cactice closed this as completed May 26, 2021
@edsrzf
Copy link
Collaborator

edsrzf commented May 26, 2021

I think it's worth leaving this open. It doesn't sound right.

Are you able to share any more details about your project that might help us figure out the cause?

@edsrzf edsrzf reopened this May 26, 2021
@Cactice
Copy link
Author

Cactice commented May 27, 2021

I can't share the full code/repo but I noticed an issue with the renaming process.
It renamed .js-> .ts file, when it should have renamed .js -> .tsx file.
As a result it was leaving lots of invalid comments // on valid React JSX syntax.
Example:


  return (
    // @ts-expect-error ts-migrate(2749) FIXME: 'MenuWrapper' refers to a value, but is being used... Remove this comment to see the full error message
    <MenuWrapper>
      // @ts-expect-error ts-migrate(2749) FIXME: 'MainMenu' refers to a value, but is being used as... Remove this comment to see the full error message
      <MainMenu>
        // @ts-expect-error ts-migrate(2304) FIXME: Cannot find name 'div'.
        <div className="mx-auto">
          // @ts-expect-error ts-migrate(2749) FIXME: 'Component' refers to a value, but is being used a... Remove this comment to see the full error message
          <Component {...pageProps} />
        </div>
      </MainMenu>

I had to manually revert the changes to .ts files, rename them to .tsx and rerun migration for those files.

Mac OS: v10.15.7
Node: v15.8.0
ts-migrate: 0.1.19 to rename, took too long for the full migration and switched to 0.1.15 to see if it helped which took 1.5 hours to finish.

@edsrzf
Copy link
Collaborator

edsrzf commented Jun 5, 2021

Strange, this sounds like it could be a limitation in the jsFileContainsJsx function in the rename command:

function jsFileContainsJsx(jsFileName: string): boolean {
const contents = fs.readFileSync(jsFileName, 'utf8');
return /(from ['"]react['"]|@jsx)/.test(contents) && /<[A-Za-z>]/.test(contents);
}

(It does look a little brittle, to be fair.)

Do you import React using require, perhaps? Or maybe you use the new JSX transform that doesn't require importing React?

@tervay
Copy link

tervay commented Jun 23, 2021

Also seeing this using the React 17 JSX transform that does not require importing React (explicit-any took 90 minutes). I think @edsrzf seems to have the best clue into why with jsFileContainsJsx.

@Rudeg Rudeg added enhancement New feature or request question Further information is requested labels Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants