[nightly][regression] Some emitted imports are syntactically invalid

[MichaelMitchell-at]

🔎 Search Terms

import, declaration, empty

🕗 Version & Regression Information

• Bisects to Replace addUndefined in serializeTypeForDeclaration with the actual declaration #58085 by @weswigham

⏯ Playground Link

No response

💻 Code

n/a

🙁 Actual behavior

A type referenced in a declaration file is sometimes emitted as import().SomeType and sometimes as import(<actual_import_path>).SomeType

🙂 Expected behavior

The type is always emitted as import(<actual_import_path>).SomeType

Additional information about the issue

It's not easy for me to provide a self-contained reproduction, as it involves the composition of a lot of our private code, but I figure since it's already known which PR introduced the regression, it should hopefully be easier for someone on the TS team to spot how an import could possibly be empty. I can patch speculative fixes or add some debug logs if requested.

[MichaelMitchell-at]

Ok I have a repro. It actually demonstrates an even more egregious behavior
https://github.com/MichaelMitchell-at/repro_for_typescript_issue_58159

export declare const d: {
    e: {
        f: (foo: import({e};
        ).Foo) => boolean;
    };
};

This is even less syntactically valid!

[weswigham]

Thanks for the repro - looks like we could have triggered this bug before #58085, but that change made it much easier to trigger, since we reuse type nodes so much more. #58165 is up with the fix.

[jakebailey]

It's not easy for me to provide a self-contained reproduction, as it involves the composition of a lot of our private code, but I figure since it's already known which PR introduced the regression, it should hopefully be easier for someone on the TS team to spot how an import could possibly be empty. I can patch speculative fixes or add some debug logs if requested.

FWIW https://www.npmjs.com/package/every-ts may be useful in the future if you're already able to use nightly TS (and can bisect instead).

[MichaelMitchell-at]

@jakebailey that's how I found the offending commit :). BTW could I put in a quick feature request for a command to rebuild TS in the current branch? Bisecting took extra work because there was an unrelated bad commit that I had to revert on every commit I was testing, so my flow was like:

every-ts bisect good
git revert ...
# copy the current revision with the revert
git checkout HEAD^  # have to switch otherwise every-ts won't rebuild TypeScript
every-ts switch <copied revision with revert>

What I'd want is something like

every-ts bisect good
git revert ...
every-ts rebuild # or alternatively something like `every-ts switch ... --rebuild`

[jakebailey]

Oh, oops, I didn't even see that you bisected it; I skipped straight to the end.

BTW could I put in a quick feature request for a command to rebuild TS in the current branch? Bisecting took extra work because there was an unrelated bad commit that I had to revert or every commit I was testing, so my flow was like:

At the moment, I'm not intending for anyone to be modifying the TS repo that every-ts checks out internally.