esbuild-plugins icon indicating copy to clipboard operation
esbuild-plugins copied to clipboard

Some errors are not output into the diagnostics due to incorrect assumption about `messageText`

Open adamscybot opened this issue 11 months ago • 2 comments

This took me a while to pin down, but I have discovered that sometimes (specific types of error) messageText from TS diagnostics is not a string, it is actually an object with nesting:

{
    // ...stuff
    "start": 335,
    "length": 15,
    "code": 2322,
    "category": 1,
    "messageText": {
      "messageText": "Type '(thing: ThingInput) => Promise<{ thingName: string; thingId: string; }>' is not assignable to type 'EntityCreator<ThingInput>'.",
      "category": 1,
      "code": 2322,
      "next": [
        {
          "messageText": "Type 'Promise<{ thingName: string; thingId: string; }>' is not assignable to type 'Promise<NameAndId>'.",
          "category": 1,
          "code": 2322,
          "next": [
            {
              "messageText": "Type '{ thingName: string; thingId: string; }' is missing the following properties from type 'NameAndId': name, id",
              "category": 1,
              "code": 2739
            }
          ]
        }
      ]
    }
}

Because of this, this check in the worker means those diagnostics are not output in the errors array.

      if (typeof messageText !== "string")
        continue;

Notably the output key does have the error in since its not subject to these checks and the errorCount is correct.

I believe this could be fixed by using flattenDiagnosticMessageText

adamscybot avatar Mar 05 '24 15:03 adamscybot

Good catch! I'll look into it

jgoz avatar Mar 05 '24 15:03 jgoz

I've patched it locally with this in reporter.ts

  private static *transformDiagnostics(
    basedir: string,
    diagnostics: readonly ts.Diagnostic[],
  ): Iterable<EsbuildDiagnosticMessage> {
    for (const diagnostic of diagnostics) {
      const type =
        diagnostic.category === ts.DiagnosticCategory.Error
          ? 'error'
          : diagnostic.category === ts.DiagnosticCategory.Warning
            ? 'warning'
            : undefined;

      if (!type) continue;

      const { file, length, start } = diagnostic;
      const messageText = ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n\n')
      if (!file) continue;
      if (typeof messageText !== 'string') continue; // Maybe dont need this check anymore?
   // Rest of it

Which works, though I dont know if you'd rather return the structure instead of squishing it into a string.

I think it'd also need applying in case 6194 on reportSummaryDiagnostic.

adamscybot avatar Mar 05 '24 21:03 adamscybot

Thanks for the patch, I applied it in @jgoz/[email protected]. Please give it a try!

jgoz avatar May 11 '24 03:05 jgoz