-
Notifications
You must be signed in to change notification settings - Fork 730
Description
This is an extension of #1685
Given this test case:
// @module: nodenext
// @target: esnext
// @outDir: ./out
// @allowJs: true
// @checkJs: true
// @noUnusedLocals: true
// @noUnusedParameters: true
// @declaration: true
// @moduleDetection: legacy
// @filename: test.js
/** @type {MyType} */
const obj = { a: 42, b: "hello" };
console.log(obj);
// @filename: types.js
/** @typedef {{ a: number, b: string }} MyType */These files are not modules, they are scripts (per moduleDetction: legacy, but auto also works). The typedef is not an export, it's declaring a global type the other file can reference.
//// [test.d.ts]
/** @type {MyType} */
declare const obj: MyType;
//// [types.d.ts]
type MyType = {
a: number;
b: string;
};But, in Corsa, the reparser turns typedef into export type, leading to a different output:
//// [test.d.ts]
/** @type {MyType} */
declare const obj: MyType;
//// [types.d.ts]
export type MyType = {
a: number;
b: string;
};
/** @typedef {{ a: number, b: string }} MyType */And an error:
test.js(1,12): error TS2304: Cannot find name 'MyType'.
==== test.js (1 errors) ====
/** @type {MyType} */
~~~~~~
!!! error TS2304: Cannot find name 'MyType'.
const obj = { a: 42, b: "hello" };
console.log(obj);
==== types.js (0 errors) ====
/** @typedef {{ a: number, b: string }} MyType */
This is all a problem, because the reparser does this transformation before we determine if a file is a module or not, and because it changes the declaration emit.
Strada avoids this by not doing anything at all with JSDoc comments, until special cases in the checker, declaraion emit, etc.
This looks like a situation similar to the one that required us to introduce ExportAssignment vs JSExportAssignment.
But, I am looking, and we already have KindJSTypeAliasDeclaration. So this may actually just come down to us doing module detection wrong in the face of these? Or something? Some other bug?