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

isolatedDeclaration errors don't include line numbers #3913

Closed
MichaelMitchell-at opened this issue Jun 26, 2024 · 6 comments · May be fixed by #3934
Closed

isolatedDeclaration errors don't include line numbers #3913

MichaelMitchell-at opened this issue Jun 26, 2024 · 6 comments · May be fixed by #3934
Assignees
Labels
C-bug Category - Bug P-high Priority - High

Comments

@MichaelMitchell-at
Copy link

Pretty self-explanatory

@MichaelMitchell-at MichaelMitchell-at added the C-bug Category - Bug label Jun 26, 2024
@Dunqing
Copy link
Member

Dunqing commented Jun 26, 2024

What do you need line numbers for, Currently our errors have full error information, usually you just print out the error

@MichaelMitchell-at
Copy link
Author

If you have a file of non-trivial size, e.g.

export const a = 1;
export const b = 2;
export const c = 3;
export const d = 4;
export const e = 5;
export const f = 6;
export const g = 7;
export const h = 8;
export const i = 9;
export const j = 10;
export const k = 11;
export const l = 12;
export const m = 13;
export const n = 14;
export const o = 15;
export const p = 16;
export const q = 17;
export const r = 18;
export const s = 19;
export const t = 10*2;
export const u: unknown = 21;
export const v: unknown = 22;
export const w: unknown = 23;
export const x: unknown = 24;
export const y: unknown = 25;
export const z: unknown = 26;

Then knowing that there's an error doesn't really help you identify where the error is:

{
  sourceText: 'export declare const a = 1;\n' +
    'export declare const b = 2;\n' +
    'export declare const c = 3;\n' +
    'export declare const d = 4;\n' +
    'export declare const e = 5;\n' +
    'export declare const f = 6;\n' +
    'export declare const g = 7;\n' +
    'export declare const h = 8;\n' +
    'export declare const i = 9;\n' +
    'export declare const j = 10;\n' +
    'export declare const k = 11;\n' +
    'export declare const l = 12;\n' +
    'export declare const m = 13;\n' +
    'export declare const n = 14;\n' +
    'export declare const o = 15;\n' +
    'export declare const p = 16;\n' +
    'export declare const q = 17;\n' +
    'export declare const r = 18;\n' +
    'export declare const s = 19;\n' +
    'export declare const t: unknown;\n' +
    'export declare const u: unknown;\n' +
    'export declare const v: unknown;\n' +
    'export declare const w: unknown;\n' +
    'export declare const x: unknown;\n' +
    'export declare const y: unknown;\n' +
    'export declare const z: unknown;\n',
  errors: [
    'TS9010: Variable must have an explicit type annotation with --isolatedDeclarations.' // which line is wrong?
  ]
}

Compare that with tsc's diagnostics:

@Dunqing
Copy link
Member

Dunqing commented Jun 26, 2024

I tested it with your code. The error looks like this.

[
  '  x TS9010: Variable must have an explicit type annotation with\n' +
    '  | --isolatedDeclarations.\n' +
    '    ,-[packages/server-renderer/src/helpers/ssrVModelHelpers.ts:22:14]\n' +
    ' 21 | export const s = 19;\n' +
    ' 22 | export const t = 10*2;\n' +
    '    :              ^\n' +
    ' 23 | export const u: unknown = 21;\n' +
    '    `----\n'
]

@MichaelMitchell-at
Copy link
Author

MichaelMitchell-at commented Jun 26, 2024

Using oxc-transform v0.15.1, this is what I see in a Node REPL:

require('oxc-transform').isolatedDeclaration('hello.ts', `
... export const a = 1;
... export const b = 2;
... export const c = 3;
... export const d = 4;
... export const e = 5;
... export const f = 6;
... export const g = 7;
... export const h = 8;
... export const i = 9;
... export const j = 10;
... export const k = 11;
... export const l = 12;
... export const m = 13;
... export const n = 14;
... export const o = 15;
... export const p = 16;
... export const q = 17;
... export const r = 18;
... export const s = 19;
... export const t = 10*2;
... export const u: unknown = 21;
... export const v: unknown = 22;
... export const w: unknown = 23;
... export const x: unknown = 24;
... export const y: unknown = 25;
... export const z: unknown = 26;`)
{
  sourceText: 'export declare const a = 1;\n' +
    'export declare const b = 2;\n' +
    'export declare const c = 3;\n' +
    'export declare const d = 4;\n' +
    'export declare const e = 5;\n' +
    'export declare const f = 6;\n' +
    'export declare const g = 7;\n' +
    'export declare const h = 8;\n' +
    'export declare const i = 9;\n' +
    'export declare const j = 10;\n' +
    'export declare const k = 11;\n' +
    'export declare const l = 12;\n' +
    'export declare const m = 13;\n' +
    'export declare const n = 14;\n' +
    'export declare const o = 15;\n' +
    'export declare const p = 16;\n' +
    'export declare const q = 17;\n' +
    'export declare const r = 18;\n' +
    'export declare const s = 19;\n' +
    'export declare const t: unknown;\n' +
    'export declare const u: unknown;\n' +
    'export declare const v: unknown;\n' +
    'export declare const w: unknown;\n' +
    'export declare const x: unknown;\n' +
    'export declare const y: unknown;\n' +
    'export declare const z: unknown;\n',
  errors: [
    'TS9010: Variable must have an explicit type annotation with --isolatedDeclarations.'
  ]
}```

@MichaelMitchell-at
Copy link
Author

I think besides the ones I've already reported, the only potential bugs remaining are the ones I haven't been able to diagnose yet without more specific error information. Looks like it's pretty close though :)

@Boshen Boshen added the P-high Priority - High label Jul 2, 2024
@Boshen
Copy link
Member

Boshen commented Jul 5, 2024

Changed in 150f4d9

@Boshen Boshen closed this as completed Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category - Bug P-high Priority - High
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants