Strange declarations created within typescript-in-jsdoc

[vankop]

TypeScript Version: 3.7.2

Search Terms:

• typescript-in-jsdoc
• jsdoc
• 3.7
• declaration

Code

https://github.com/webpack/schema-utils/blob/72d2ea1b6025f0069b5bde49ea057df376c22057/src/validate.js

Expected behavior:

Expect correct declaration file created

Actual behavior:

https://github.com/webpack/schema-utils/blob/7b41f1ec6f0be3ecd7b023bdd3d6a5db052bbee3/dist/validate.d.ts

Got strange export:

declare namespace validate {
    export { _default as ValidationError, _default as ValidateError };
}

If you will take a look on source code, it does not have _default export and ValidateError export either

Related Issues:

downstream issue: webpack/schema-utils#75

[Bnaya]

probably related do #33626

[weswigham]

Strange - I don't even know why the validate default export has a namespace meaning attached to it - let alone members named ValidationError or ValidateError. It looks like something might be wonky in how we check that function.

[Bnaya]

ValidationError is possible throwed from inside validate, so i would guess it's associated with it somehow

BTW if you would add in the source file of validate.js, it will fix the d.ts file,

/** @typedef {import("./ValidationError").default} _default */

The namespace export won't be gone, but _default won't be missing

[weswigham]

ValidationError is possible throwed from inside validate, so i would guess it's associated with it somehow

We don't track exceptions in any way, so that shouldn't be it. As I said - I'll need to look into it, something strange is going on.

[weswigham]

Ah, I see. We're picking up the additions made in index.js and trying to merge them back down into the original site we found the thing at (which doesn't work great, since the sources of those pseudoaliases is in a different file).

[weswigham]

So, I can provide a quick fix that elides the buggy declarations from the file, as they're fairly easy to detect, but the shape is quite troublesome to serialize with fidelity:

For a full and complete fix that includes all the information from the assignments of index.js in index.d.ts, I think we'll need two relatively large root issues in our JS checking fixed:

1. Binding const x = require("...") as an alias where possible (so we have a chain of aliases to record how a symbol was located in JS (hopefully reducing the need for or removing the js "type of a value" fallback name resolution codepath), and on which we can hang...)
2. The ability for an alias symbol to record merges to the target of the alias. Right now in JS, this is tacitly allowed via how module.exports merges are performed (where everything is ad-hoc merged into the alias target), but we have no way of knowing what has contributed to a merge like this in the file the merge occurred in - formalizing and properly encoding these merges so the system can be aware of them, and generalizing them to the point where something like

import mod from "mod";
export default mod;
mod.MyMember = class MyMember {
  // ...
}

emits as

import mod from "mod";
export default mod;
class MyMember {
  // ...
}
declare module "mod" {
    export namespace default {
      export { MyMember }
    }
}

and would be valid in TS (rather than throwing with something like const mod has no member named "MyMember"). Do note that export namespace default (or export default namespace) is also not valid today - there is no way to augment a default today unless the thing default exported is an alias and the target of that alias is accessible to be augmented, which is also problematic.

[weswigham]

@Bnaya @vankop as a workaround until we have a fix, if you move the augmentating assignments into the file with the original declaration, we should do a little better. So rather than

module.exports.ValidationError = ValidationError.default;

// Todo remove this in next major release
module.exports.ValidateError = ValidationError.default;

in index.js, write

validate.ValidationError = ValidationError.default;

// Todo remove this in next major release
validate.ValidateError = ValidationError.default;

in validator.js instead. The cross-file mutation is tripping us up and we don't have a great way to represent that right now - I'm surprised we actually type check it correctly.

[vankop]

@weswigham I think we can move export default function validate on top instead of function validate() {} export default validate;, see how it works in downstream issue. Maybe I am wrong in this idea, but declarations looks fine =)

[hinell]

@weswigham It seems like that the --module compiler option doesn't affect the declarations generation.
In specified sources above the option is set to es6 (by default) meaning that instead of namespaces must be generated modules.