#Experimental Typescript Types.

1 messages · Page 1 of 1 (latest)

vast horizon
#

threaded for conversation.

#

Tag @rigid zinc @quaint nest @wide hollow, @verbal quartz

wide hollow
#

This would be a huge leg up on the current effort 😂 that would hopefully let the TS project focus on figuring out the hard translations - namely, the fricken dataModel

rigid zinc
#

Firstly I want to say thank you for reaching out. I'd certainly be willing to review the .d.mts and figure out how to make sure it's a part of your tooling that they're correct. Any places where the documentation doesn't match up against reality have usually ended up being mild headaches.

verbal quartz
#

Yeah the datamodel, especially with regards to infering types from schemas has been a major long time project of f1r3 especially

#

That said, any help from foundry itself is always welcome :D

#

And I'm happy to see you guys take this step :D

rigid zinc
#

Personally I spent months on it back in 2022. The issue, surprisingly, isn't the direct conversion of data models to Typescript types but in the fact that it seems to run into problems like inexplicable infinite recursion that's been very hard to debug

verbal quartz
#

Anyway, I'd be more than happy to help out here and give feedback :D

#

How exactly will the types be made available? Do you plan to publish them as an npm package like the CLI?

rigid zinc
#

Even if it's just something that we'd have to manually do like tsc --emitDeclaration on the code base or whatever tool you want to use that's fine imo.
I think the main benefit of this files will being able to use them to diff Foundry versions more efficiently.

#

When people are updating the types they have three bits of code up:

  1. The current Foundry version of the types
  2. The Foundry version that the types are being updated to
  3. The current codebase of types
#

If you want to be maximally careful you're actually reading both Foundry versions line by line, including stuff like function bodies. We might not have to type anything in a function body (just the function signature) but subtle changes in the body can reveal that the function signature has actually changed without the jsdoc being updated. I'd struggle to point to a specific example off the top of my head but if a function adds a line like return true; obviously true is now one of the valid return values.
Now that's been rarer and rarer but diffing would be more efficient if we had to compare two .d.ts files that we can assume are correct.

vast horizon
#

Yes, the ability for the declarations to be diffed should be a big advantage to the community wanting better TS types

#

As a starting point I'd like to focus on the new client-side ESM module which nests the prior commons module while adding new client-specific functionalities.

#

This ignores the monolithic foundry.js client code for now and focuses on a more narrow subset of code where we believe we are documenting it "well"

#

our goal will be to gradually move more client code to the standard

#

Please feel free to ping me with any findings or questions.

#

Some good example files to look at would be ones in the new v12 foundry.sound submodule.

rigid zinc
#

Okay so the first thing I'm definitely going to do is run tsc on this with allowJs: true. In an ideal world this type checks because anywhere there isn't a jsdoc everything will just be any.
Then the next thing I'm going to do is tsc --emitDeclarationOnly to generate .d.ts files for this folder.

I'll manually take a look at the .d.ts files and see how they look in terms of diffability.

#

I think unfortunately it's unlikely to type check and unlikely to generate value .d.ts files

vast horizon
#

I think you may be misunderstanding.

#

These are the generated .d.ts files

#

They were generated with the following tsconfig.json:

{
  "files": [
    "client-esm/client.mjs",
    "public/scripts/foundry.js",
    "client/hooks.mjs",
    "prosemirror/prosemirror.mjs"
  ],
  "compilerOptions": {
    "allowJs": true,
    "declaration": true,
    "emitDeclarationOnly": true,
    "outDir": "./_dts"
  }
}
rigid zinc
#

ah my bad, I read .mjs instead of .mts when you were typing up your description

#

(working with .mts files is relatively rare for me because those are just .ts files these days)

vast horizon
rigid zinc
#

yeah it's not a problem, just explaining my screw up in reading comprehension

#

Kudos that this builds as well as it does. It doesn't build for a few reasons but I didn't expect it to.

I'm going to focus on client-esm rather than common because client-esm is newer and common has a lot of older legacy problems.

Here's the general type of thing that Foundry could do to improve these definitions:

Firstly here's things that'll help regular users who want to browse Foundry documentation:

  • Function is used but it basically describes any function. It'd be more useful to have something like (x: number) => string. You can even document what the parameters of the callback mean this way.
  • There's stuff like DataFieldOptions and FilePathFieldOptions not being defined
  • There's parameters that are left with any. If that's intentional and to be worked on later, that's fine but it basically means it's undocumented in effect.
  • To make sure the jsdoc matches actual code, you could make sure that it's a part of your ci/development so your developers are being told if they forget to update the types in jsdoc but update the function. Maybe you do this already!

Here's some things that probably have less broad implications but if they're solved it'll help us a bit. Some are nitpicks, some would be a bit important.

  • The type object allows you to assign arrays to them. As in you can do const x: object = [1, 2, 3]; because an array is an object.
    Where you use the type object you usually mean Record<string, unknown>. A Record<string, unknown> is an object (NOT an array) where the keys are strings and the values are unknown. So you CAN'T do const x: Record<string, unknown> = [1, 2, 3]; but you CAN do const x: Record<string, unknown> = { foo: 1, bar: "2", lorem: [3, 4, 5] };
  • Class extension in Typescript is fairly strict so this code ends up with errors like
Property 'type' in type 'BiquadFilterEffect' is not assignable to the same property in base type 'BiquadFilterNode'.
  Type 'string' is not assignable to type 'BiquadFilterType'.
  • The same thing happens with methods. For example AudioBufferCache.set is typed this way set(src: string, buffer: AudioBuffer): AudioBufferCache; but it should be typed as set(src: string, buffer: AudioBuffer): this; because that's how Map<any, any> (which AudioBufferCache extends) types it.
  • Speaking of AudioBufferCache, it should probably extend Map<string, AudioBufferCacheEntry> or Map<string, AudioBuffer> not Map<any, any>. I'm not sure which is correct because both types are used.
  • I have no idea what static "__#28@#nodeId": number; and __#28@#unloadAudioElement in Sound is. Is it some true private mangling or something? It's really not important but interesting.
vast horizon
#

Good comments @rigid zinc - thanks for this review. There's definitely some action items we can take here.

Function is used but it basically describes any function. It'd be more useful to have something like (x: number) => string. You can even document what the parameters of the callback mean this way.
Yeah, this is admittedly a little lazy. We can work to remove these in favor of defined callbacks or inline function annotations.

#

There's stuff like DataFieldOptions and FilePathFieldOptions not being defined
Not sure what you are highlighting here. The typedef is defined and exported in common/data/fields.d.mts. Is there something wrong with that type export?

#

There's parameters that are left with any. If that's intentional and to be worked on later, that's fine but it basically means it's undocumented in effect.
There is only one use of {any} in client-esm that I am able to see right now and its in the SoundScheduleCallback. The callback function can return anything it wants to, and the return value is forwarded on, so the return type is actually any.

rigid zinc
#

It's exported just fine, just not imported in data.d.mts

vast horizon
#

yeah, there's no such thing as importing @typedefs in javascript, they are just declared and used globally.

#

so I guess the tsc build doesn't figure out to import them

rigid zinc
#

Ah I thought it'd look like import("...").DataFieldOptions

vast horizon
#

I'm not sure how to resolve that.

rigid zinc
#

well if it doesn't matter for the Foundry documentation it doesn't really matter for us either

#

As in if the website will hyperlink to the appropriate type

hot lotus
#

this would make us at the OSE (and related systems) project very happy

vast horizon
#

it probably wont, because the docs are typedoc.

#

JavaScript has no concept of a declare type, this is supported through docstring annotations like:

/**
 * @typedef {Object} DataFieldOptions
 * @property {boolean} [required=false]   Is this field required to be populated?
 * @property {boolean} [nullable=false]   Can this field have null values?
 * @property {Function|*} [initial]       The initial value of a field, or a function which assigns that initial value.
 * @property {Function} [validate]        A data validation function which accepts one argument with the current value.
 * @property {string} [label]             A localizable label displayed on forms which render this field.
 * @property {string} [hint]              Localizable help text displayed on forms which render this field.
 * @property {string} [validationError]   A custom validation error string. When displayed will be prepended with the
 *                                        document name, field name, and candidate value.
 */
#

There's no importing or exporting of them, since they aren't actually objects in the module, they are documentation-only.

rigid zinc
#

This answer points out that import(...) syntax is a Typescript extension

#

So yeah it should work fine on the documentation website

#

which is what matters

#

in this case

vast horizon
#

interesting. Was not aware of that. I will do a test

rigid zinc
#

If writing out the whole import each time gets a bit annoying you can do /** @typedef {import(...).Type} Type */

vast horizon
#

sorry, I think I'm missing the difference between what you are saying here and the "whole import"

rigid zinc
#

Say you use the type several times in a file, like so:

/** @type {import("./foo").Bar} */
const x = ...;

/** @type {import("./foo").Bar} */
const y = ...;

/** @type {import("./foo").Bar} */
const z = ...;

It might be more convenient to do:

/** @typedef {import("./foo").Bar} Bar */

/** @type {Bar} */
const x = ...;

/** @type {Bar} */
const y = ...;

/** @type {Bar} */
const z = ...;
#

(I can't guarantee this is valid code, I've used this trick before but I didn't test this specific code atm)

vast horizon
#

that is what is shown in the SO example you linked, using the import to define a typedef

#

It is, unfortunately not working though.

#
/**
 * @typedef {import("../fields.mjs").DataFieldOptions} DataFieldOptions
 */

In the common/data/data.mjs file where the DataFieldOptions typedef is declared in common/data/fields.mjs.

#

Just produces the following when I export the types:

export type DataFieldOptions = any;
export type FilePathFieldOptions = any;
#

doesn't seem to be working as advertised on the SO post.

rigid zinc
#

Bear in mind a few unresolved types isn't the end of the world for us or anything

#

The fact that they exist at all should largely be good enough

#

I started venturing down this because I wasn't sure if it'd display wrong on the documentation website as well

vast horizon
rigid zinc
#

ah! Good, then I hit upon something important

#

I just didn't want to get you down this rabbit hole under the impression I needed it

vast horizon
#

unfortunately the import() technique is not working, unless I'm implementing it incorrectly.

rigid zinc
#

Yeah the import() technique is a Typescript extension so it won't work everywhere

#

What does your documentation site run on if I might ask?

vast horizon
rigid zinc
#

Ah that's interesting... that definitely should support import(), what's the converter to markdown (or whatever) you use?

vast horizon
#

no such thing

#

its more direct than you are envisioning

#

we run typedoc directly using typedoc.json:

{
  "$schema": "https://typedoc.org/schema.json",
  "name": "Foundry Virtual Tabletop - API Documentation - Version 12",
  "entryPoints": [
    "common/types.mjs",
    "common/primitives/module.mjs",
    "client/hooks.mjs",
    "client-esm/client.mjs",
    "public/scripts/foundry.js"
  ],
  "out": "_docs",
  "categorizeByGroup": true,
  "defaultCategory": "- Other",
  "disableSources": true,
  "hideGenerator": true,
  "sort": ["instance-first", "visibility"],
  "media": "docs/media",
  "customCss": "docs/css/typedoc.css",
  "includes": ["docs/md"],
  "excludeExternals": true
}

Which in turn runs typescript build with tsconfig.json as:

{
  "files": [
    "client-esm/client.mjs",
    "public/scripts/foundry.js",
    "client/hooks.mjs"
  ],
  "compilerOptions": {
    "allowJs": true,
    "declaration": true,
    "emitDeclarationOnly": true,
    "outDir": "./_dts"
  }
}

The docs are built from the generated .d.ts files.

tall pebble
#

Yeah.. So I have a lot of experience w/ ESM and generating types. Indeed import types can be a bit of a pain.

The suggestion of local typedefs ala:

/**
 * @typedef {import("../fields.mjs").DataFieldOptions} DataFieldOptions
 */

does not work well for various reasons.

I'd be glad to offer input on generating types from ESM though as I've put a lot of effort into this area in my own work.

#

There are general problems just running ESM through tsc as well that need to be worked around.

vast horizon
#

@tall pebble please feel free to suggest. We would like to offer these types for the community benefit. We have no real need of them internally.

tall pebble
#

It'd help out everyone for sure so definitely excited to see this engagement!

#

So, I have put in a lot of time on tooling that does use the Typescript compiler and several other key packages to make an easier go of making the process turnkey.

I'll definitely take a stab at running things through this tooling once the v12 prototype drops.

https://www.npmjs.com/package/@typhonjs-build-test/esm-d-ts

Not just trying to promote this here. This is the key tooling I use to generate extensive types for all my ESM efforts.

The next big feature I'm adding to this tooling is a project analysis stage that can automatically handle locally linked symbols without the need for import types. In all of my work presently I use import types for everything and that is not easy for those used to "normal" JSDoc.

rigid zinc
#

Notably import type would require the code itself to be compiled, to compile away the import type.

#

(unless Typhon is talking about something else)

#

which is why it's not easy

#

Haha, I was looking through the Typedoc repo to see if anyone else had run into these import issues

tall pebble
#

I don't want to get into the weeds per se, but yes this project analysis stage would synthetically generate the import types as needed. The tooling supports intermediate AST transformation.

There are some tricky things right now that tsc doesn't generate good types for like static private methods / variables in ESM that are handled by transformers in my tooling.

vast horizon
#

it feels to me like if typescript wants to offer an allowJs compilation option and operate under the presumption that "typescript is a superset of JS" they need to fix some things in these areas.

#

Things like lack of support for static private fields, which @rigid zinc asked about in:

I have no idea what static "__#28@#nodeId": number; and __#28@#unloadAudioElement in Sound is. Is it some true private mangling or something? It's really not important but interesting.
Being an example.

tall pebble
#

That is indeed the kind of thing I'm talking about. tsc stumbles on a few things (static private fields) that will look just like the example above.

verbal quartz
vast horizon
#

but hopefully we can figure out some improvements together that get the core exported types "as close as realistically possible" to what the typescript developer community would want.

#

And then further adjustment of those types can be done by hand.

vast horizon
rigid zinc
#

Interestingly I'm getting import to work

#

here's my reproduction

vast horizon
vast horizon
verbal quartz
#

ah, I see

rigid zinc
#

Please:

  1. Unextract the zip
  2. Open a terminal inside it and...
  3. Run npm install
  4. Run npx tsc

I get a _dts/foo.d.mts file with this, correct, content:

/** @type {import("./bar.mjs").Bar} */
export const x: import("./bar.mjs").Bar;
#

The main change I made was "include": "**/*.mjs" in the tsconfig.json

tall pebble
#

import types does work. It's just tedious in respect to normal JSDoc use cases.

You don't need extensions in the import types.

IE this should work:

{import("../fields").DataFieldOptions}
rigid zinc
#

Yeah, I had Atropos try that

#

It was being inferred/generated as any.

#

That was the problem I was trying to solve with him

vast horizon
#

@rigid zinc do you have knowledge of the difference in tsconfig directives between files vs include? You mention in your example you configured it to include all .mjs files. Our tsconfig.json is configured with a specific set of files as entry-points.

rigid zinc
#

Yeah so if I were to attempt to TLDR the concept, files is exactly what you described, entrypoints

#

that means it performs tree shaking

#

Unfortunately in this case, it means it can no longer include arbitrary JSDoc imports without an associated TS or maybe JS import

#

Because there's no "link" there

#

includes says "all these files should be compiled regardless"

#

and so they can find the "link" between all the files, hopefully no matter what

#

I know that probably doesn't look like a TLDR considering it's like a paragraph but the distinction is just obnoxiously subtle

verbal quartz
#

treeshake vs not treeshake?

rigid zinc
#

basically

#

but includes doesn't suddenly turn off tree shaking entirely

#

it still doesn't bundle the files unnecessarily, or at least it shouldn't always

#

But that's a close enough approximation here

verbal quartz
#

juuuuuuust to be sure this sanity question has been asked:
you guys DID look at the typescript handbook pages for generating types from JS, yes?

rigid zinc
#

dunno if Atropos has but generally speaking I've read everything on the typescriptlang.org website

#

I'm able to get the imports working

#

just trying to troubleshoot why/if Atropos can't

verbal quartz
#

right, sorry, yes the question was mostly directed at Atropos.

rigid zinc
#

Ah no problem

#

I wasn't offended

#

tone is hard over text but I was actually mostly laughing at myself with the line "I've read everything on the typescriptlang.org website" because of course that's a bit of a silly thing to have done, I just got bored and read the site mostly front to back

verbal quartz
#

I'm not even suprised you've done that.

#

You're very technically skilled, so knowing you ACTUALLY devoured the handbook just makes me respect you even more 😄

rigid zinc
#

haha, there's a lot of things in there I think would be useful to even very experienced Typescript devs, it's how I finally figured out that T extends U ? ... : ... distributing based upon whether T is generic or concrete was actually intended and not some weird bug I had to always be latently aware of. But I digress.

#

Ah I suppose the simplest way to explain it in this case would probably be "includes supports globs but files doesn't"

#

That's all that'll matter for you

#

You could put everything in includes if you wanted in this case

tall pebble
#

Anyway.. I think it is worth a shot of trying out esm-d-ts as it handles a lot of the TS voodoo required. You give it the ESM entry point source file and it does the rest. No tsc configuration necessary.

vast horizon
#

@rigid zinc half-progress. I can switch to:
tsconfig.json

{
  "include": [
    "client-esm/**/*.mjs",
    "public/scripts/foundry.js",
    "client/hooks.mjs"
  ],
  "compilerOptions": {
    "allowJs": true,
    "declaration": true,
    "emitDeclarationOnly": true,
    "outDir": "./_dts"
  }
}

And in data.mjs:

  /**
   * @param {import("../fields.mjs").DataFieldOptions} options  Options which are forwarded to the SchemaField constructor
   * @param {import("../fields.mjs").FilePathFieldOptions} srcOptions   Additional options for the src field
   */

Which ends up preserving that import type in the generated data.d.mts file:

    /**
     * @param {import("../fields.mjs").DataFieldOptions} options  Options which are forwarded to the SchemaField constructor
     * @param {import("../fields.mjs").FilePathFieldOptions} srcOptions Additional options for the src field
     */

buuuuttttt it all breaks down when generating the typedoc which produces the following documentation for the TextureData constructor:

rigid zinc
#

drats

vast horizon
#

it's a strange result. options comes out just as any, while srcOptions somehow at least has the name of the type correct, but it's not properly linked.

#

But maybe I need different typedoc.json configuration.

verbal quartz
#

likely

vast horizon
tall pebble
#

Try it without the extension. import("../fields").DataFieldOptions

rigid zinc
#

Can you show one more line in data.d.mts? I want to see the constructor itself in the data.d.mts

#

I see just the documentation right now

vast horizon
#
export class TextureData extends fields.SchemaField {
    /**
     * @param {import("../fields.mjs").DataFieldOptions} options  Options which are forwarded to the SchemaField constructor
     * @param {import("../fields.mjs").FilePathFieldOptions} srcOptions Additional options for the src field
     */
    constructor(options?: any, { categories, initial, wildcard, label }?: any);
}
#

so yeah, did not get properly attributed

vast horizon
rigid zinc
#

Yeah if the .d.mts has any the doc should too I suppose

vast horizon
# tall pebble Try it without the extension. `import("../fields").DataFieldOptions`

No change:

export class TextureData extends fields.SchemaField {
    /**
     * @param {import("../fields").DataFieldOptions} options  Options which are forwarded to the SchemaField constructor
     * @param {import("../fields").FilePathFieldOptions} srcOptions Additional options for the src field
     */
    constructor(options?: any, { categories, initial, wildcard, label }?: any);
}
verbal quartz
#

okay so we can likely rule out an issue with typedoc for now

rigid zinc
#

Interestingly this is still working fine in my toy project, I'll upload it soon

verbal quartz
#

which TS versions are you using respectively

tall pebble
#

Any chance you can make available a small subset of v12 commons that is ESM oriented and I can run things over w/ my tooling and report back?

Was going to do this w/ prototype 1, but could be handy now.

verbal quartz
#

just to rule out version-specific wierdness

rigid zinc
#

v5.2.2 but it probably shouldn't matter

#

Because import(...) is extremely old

#

Atropos also got my test project working

#

Good to check nonetheless

vast horizon
#

we just declare a dev dependency on typedoc (currently 0.25.3) which includes typescript 5.2.2

rigid zinc
#

Okay dumb question

#

What's the directory structure look like?

#

I just need to know two files really

#

One actually, where is TextureData at file wise? I know fields.mjs is at src/foundry/common/data/fields.mjs in the current version

frigid fable
#

I feel obligated to also ping @quick tundra and @low blade who i believe are involved in detailing typings in some fashion, whether related to a specific system or as a past contributor

verbal quartz
#

shark is more involved on the PF2e side but having his expertise is definetly nothing bad 😄

vast horizon
#

@rigid zinc I have a reproducible failure state for you to test.

rigid zinc
#

This is my newest reproduction of working imports for DataFieldOptions

low blade
#

I'm definitely more on the past side after breaking my brain on types before and no longer having any reason to work on them anymore.

rigid zinc
#

and great!

#

...to Atropos

#

not to you haha SpiceKing, sorry about that

#

The imports are extremely picky. Well they're actually exactly as picky as normal js imports but if you screw them up it just doesn't tell you and uses any

#

I messed up the imports several times while making my reproductions

vast horizon
#

In your simple example that you provided to me please make the following changes.

  1. In bar.mjs change your typedef to be an Object with properties instead of just a Number.
/**
 * @typedef {Object} TestTypedef
 * @property {number} foo
 * @property {string} bar
 */
  1. In foo.mjs change your example use case that uses the typedef to be an exported class where the typedef is used in the constructor:
/**
 * A Test class.
 */
export default class Test {
  /**
   * Construct a Test using a typedef param.
   * @param {import("./bar.mjs").TestTypedef}
   */
  constructor(param) {
  }
}

Observe the resulting output foo.d.mts:

/**
 * A Test class.
 */
export default class Test {
    /**
     * Construct a Test using a typedef param.
     * @param {import("./bar.mjs").TestTypedef}
     */
    constructor(param: any);
}
rigid zinc
#

Sure thing, working on it

verbal quartz
tall pebble
rigid zinc
#

ah

#

you're faster than I am

#

good job

#

Yeah that'd be the problem

vast horizon
#

hmm, oops on my end there. thanks

#

okay

rigid zinc
#

Yeah as you can see it's quite picky

verbal quartz
#

that produced a proper

/**
 * A Test class.
 */
export default class Test {
    /**
     * Construct a Test using a typedef param.
     * @param {import("./bar.mjs").TestTypedef} param
     */
    constructor(param: import("./bar.mjs").TestTypedef);
}

^^

vast horizon
#

thought I was onto something

#

guess not

rigid zinc
#

Like genuinely not trying to evangelise because I know switching to Typescript isn't feasible for you at the moment

vast horizon
#

okay... so what is wrong in our actual ESModule then?

rigid zinc
#

but this is why I don't use JSDoc atm

#

because I hate not getting warned about imports and small issues like that

rigid zinc
#

I'm pretty sure it's the import btw

vast horizon
rigid zinc
#

import("../fields") won't work either way because it doesn't have an extension but import("../fields.mjs") probably isn't working because if I had to guess TextureData isn't one directory deeper than fields.mjs or something small like that

#

or one of the jsdoc comments is slightly invalid

#

that sorta deal

quick tundra
#

I don't want to be a party pooper, but the league types aren't inhibited by errors or lack of thoroughness in typedocs

verbal quartz
#

we're keenly aware of our lack of manpower

rigid zinc
#

Yeah that's true, though at the moment we're actually troubleshooting an issue with the documentation website

#

so regardless of general priority we'll probably keep troubleshooting, just to clarify

#

but point stands in general

quick tundra
#

Sometimes a documented type turns out to be incorrect or oversimplified, but that's usually just a matter of someone doing types catching it and making corrections

#

The real hurdles aren't ones Foundry can help with: accurate and usable types for the purpose of typescript are often just hard to do and massive beyond any use to a human reader of documentation

#

But I hear you on improving the docs themselves

low blade
rigid zinc
#

Yeah! Glad to see, I thought it was

verbal quartz
#

what I am interessted in is how/if tsc will handle all the mixin and inheritance-breaking shenanigans going on

#

if at all

rigid zinc
#

we've technically handled that before, mostly by widening the base class or doing some... workarounds

#

mostly by making it not technically extend the base class anymore

#

That's unfortunately an engrained design problem though

#

A lot of overrides would have to change to follow TS's semantics

verbal quartz
#

yeah but that's all been done by hand and I don't think it's within the scope of this project to adjust the generated types or source for that

rigid zinc
#

Yeah, exactly

#

It'll probably all still be done by hand and I think that's largely fine

low blade
rigid zinc
#

It's now generating:

export class TextureData extends fields.SchemaField {
    /**
     * @param {import("./fields.mjs").DataFieldOptions} options  Options which are forwarded to the SchemaField constructor
     * @param {import("./fields.mjs").FilePathFieldOptions} srcOptions Additional options for the src field
     */
    constructor(options?: import("./fields.mjs").DataFieldOptions, { categories, initial, wildcard, label }?: import("./fields.mjs").FilePathFieldOptions);
}
vast horizon
#

in my defense. This has about 20% of my attention compared to the 80% directed towards other ongoing things.

#

but im still ashamed.

rigid zinc
#

no problem, genuinely

#

I made the same mistake while iterating on my toy examples

#

I have realised just how much I depend on intellisense

verbal quartz
#

I just wish it wouldn't us the import() in the signature, makes it hard to read but I understand why it does it

rigid zinc
#

All that matters for us atm is that it'll work for the documentation website

vast horizon
#

The generated .d.mts file looks alright

export class TextureData extends fields.SchemaField {
    /**
     * @param {import("./fields.mjs").DataFieldOptions} options  Options which are forwarded to the SchemaField constructor
     * @param {import("./fields.mjs").FilePathFieldOptions} srcOptions Additional options for the src field
     */
    constructor(options?: import("./fields.mjs").DataFieldOptions, { categories, initial, wildcard, label }?: import("./fields.mjs").FilePathFieldOptions);
}
rigid zinc
#

Let me guess, the documentation site still doesn't work?

vast horizon
#

ooh, wait, it did work. I think I refreshed too fast.

rigid zinc
#

oh! nice

vast horizon
#

🥳

low blade
vast horizon
#

I have confirmed that the following also works:

/**
 * @typedef {import("./fields.mjs").DataFieldOptions} DataFieldOptions
 */

/**
 * @typedef {import("./fields.mjs").FilePathFieldOptions} FilePathFieldOptions
 */
#

which produces more docstrings because it keeps the "ugly" import() stuff separate

low blade
#

I think you can make it one large comment block, rather than one per.

rigid zinc
#

Yeah you can!

#

I wish I wish you could just do @import or something

#

but this works

low blade
#

Yea, it's not pretty, but it does work for how verbose it is.

verbal quartz
#

works > pretty

vast horizon
#

aah, multiple in a single comment block is better, I wasn't sure whether that would be supported.

rigid zinc
#

I'm honestly tempted to see if I can literally look into the jsdoc language server and see if I can make a PR to validate imports or something...

#

As in just put in red squigglies when they're wrong

verbal quartz
#

that would be rad

rigid zinc
#

I'm inclined to believe it won't happen though, for a few reasons but majorly because simpler validations like just straight up incorrect syntax aren't validated

#

I've never found a single instance of surfaced validation in the JSDoc language server

#

(By that I mean there's obviously validation to be able to generate stuff but it just doesn't tell you about it)

#

The Typescript language server is what tells you some of the things but general JSDoc stuff isn't quite in its purview like syntax

verbal quartz
#

yeah the TS server usually just automatically runs in vscode IIRC

vast horizon
#

Hopefully now that we have a solution to one of the limitations in the .d.ts file we will be able to identify and apply this solution elsewhere that we use a typedef in a different file than the one that declares it. It will take some getting used to on our end though.

rigid zinc
#

Of course. Maybe there's a more frictionless way to do this... but I'm not aware of one

#

Are there other issues related to the types? I'm asking because there could be other "holes" in the documentation website I could take a look at

vast horizon
#

mixins are not handled well, which is a pattern we use frequently.

#

static private fields are not handled, although they don't need to be really.

wide hollow
#

Yeah the current strategy just ignores the true-private fields and methods

rigid zinc
#

*mostly

#

It technically matters every once in a blue moon

#

Mostly for niche inference reasons

#

But yeah we mostly just leave them off

wide hollow
rigid zinc
#

Can someone give an example?

#

Preferably with a link to the documentation I mean

#

And what could be added to be improved

#

This may just be a limitation in JSDoc

vast horizon
#

Mixin Documentation Challenge

@rigid zinc you can think of this more as "dynamic class definition". A simple example would be like:

/**
 * Augment a class with mixin behavior.
 * @param {typeof BaseClass}
 * @returns {typeof MyMixedClass}
 */
function MyMixin(BaseClass) {
  /** 
   * A class augmented with mixin behavior.
   * @mixin
   */
  class MyMixedClass extends BaseClass {
    
    /**
     * Add one to a provided number.
     * @param {number} n
     */
    addOne(n) {
      return n + 
    }
  }
  return MyMixedClass;
}

/**
 * Some intermediate class. Not the focus of this exercise.
 */
class A { 
  /**
   * Log a provided name to the console.
   */
  logName(name) {
    console.log(name);
  }
}

/**
 * This is the class we want to have documented.
 * The generated documentation for class B should contain both B#logName and B#addOne as instance methods.
 * @extends A
 * @mixes MyMixedClass
 */
class B extends MyMixin(A) {
}

JSDoc supports docstring annotations, but TSDoc/Typedoc does not. I've included the JSDoc tags for example above, but they don't do anything.

tall pebble
#

OK.... So, using the test sources provided by LukeAbby I found more Typescript voodoo in this process. It turns out that TSC generates incorrect types for .mjs files where .js files are fine.

I have a full demo here w/ esm-d-ts setup. The .mjs output matches the symptoms that were discussed prior and you can compare the types generated in the repo from the README:

https://github.com/typhonjs-fvtt-scratch/esm-d-ts-demo

rigid zinc
#

Ahh I knew how mixins worked and TS DOES support them to an extent but looks like what you're saying is TSDoc just doesn't recognize them

rigid zinc
#

Add the extension back in the mjs version and it'll work

#

I showed Atropos some correct imports for his example already

#

But I can fix them in your repo too if you'd like

#

It's probably that it tries adding the .js extension automatically

#

Because people are very used to that

#

But it doesn't try adding the .mjs extension for you automatically

tall pebble
#

Right you are... .mjs files w/ import types do require the extension.

rigid zinc
#

Yup, one of the many evils of webpack era

#

(take this as a tongue in cheek tone)

vast horizon
#

I have to run, but I'll leave the "mixin challenge" for folks here to noodle over.

low blade
rigid zinc
#

I'll see if I can sort anything out but it might be a feature request to TSDoc itself

rigid zinc
#

Just clarify

#

I have a love and hate relationship with webpack

#

And was making a quite sarcastic comment towards it's "evils"

#

So I figured I might as well explicitly state my tone, given that it's text

tall pebble
#

OK. Updated the demo. Anyway. This shows how to configure esm-d-ts and use it. It makes life much easier for ESM projects. Always good to get more info on TS knitpicks re: import types requiring .mjs extensions for those files.

All of my work uses .js and "type": "module" in package.json, so didn't come across that aspect.

#

The nice thing too just to mention is that esm-d-ts creates bundled type declaration output and not type declarations per source file, so some nice to have aspects there. It has even neater features and does a lot more too, but don't want to pollute this conversation.

rigid zinc
#

For Atropos when he's back: your typedoc.json has "includes": ["docs/md"], but the schema says it has to be a string not an array so I think it should be "includes": "docs/md". Doesn't seem important.
I'm working on mixin stuff but as far as I can tell, it's just not supported

tall pebble
rigid zinc
#

Oh for sure, Typescript has full support for mixins.

rigid zinc
#

because JSDoc DOES have a @mixes attribute

#

documented, as in, in the Foundry API website

rigid zinc
tall pebble
#

OK.. I know I'm talking more about esm-d-ts, but it handles the ability to include Typescript files that are referenced by JSDoc / rest of the ESM source code. For anything that is too tedious or cumbersome to create w/ JSDoc you can drop in a .ts file with the types and esm-d-ts includes them in the type declaration generation. You can reference anything from the .ts types you create including interfaces, highly templated things and whatever else in the ESM JSDoc and it just works.

quick tundra
#

I don't want to derail this thread, but sure we can talk about it over in the league channels

rigid zinc
#

I know the initial discussion of this thread was about generating declaration files

#

but the website runs off of JSDoc declarations

tall pebble
#

It can. I can give examples where I create interfaces and other things that JSDoc doesn't facilitate that are output for the types and present in the API docs.

I generate API docs for my work from the types using TypeDoc. IE you can have TypeDoc use the types instead of the .js source.

rigid zinc
#

Out of interest what would it look like for Atropos's example?

#

If it's not a hassle at least

#

seeing the final result of how it'd be documented on the site could be nice

tall pebble
#

Yep... Working on it.. I'll add an interface and setup things to generate a docs output. I also have some other really spicy TypeDoc plugins. I have created API docs for all the built-in TS libs and extended modern web APIs that can be linked to TypeDoc generation. IE full end-to-end API docs that don't stop at the project source.

You can see that TypeDoc plugin / and modern web API docs here: https://www.npmjs.com/package/@typhonjs-typedoc/ts-lib-docs

rigid zinc
#

Interesting, I'll hold off on diving into the minutae until I see the resulting documentation web page, mostly because it might not satisfy whatever Atropos is looking for

#

but hopefully it does

verbal quartz
#

I'm quite interessted to see how all of this will trickle down to community devs

#

IF (and that's a big if) the type declarations will be made available as a package it would probably help a lot of people

vast horizon
rigid zinc
#

Ah I misstated

#

I knew it was TSDoc I just didn't make the distinction when I said that

#

and yeah I agree, TSDoc is way better maintained

#

which is certainly awkward

vast horizon
rigid zinc
#

Yeah, of course!

#

I just wanted to try to point out JUST improving the declaration files wouldn't solve the mixin problem

#

unless it also fixed the documentation

vast horizon
rigid zinc
#

Well depends on whether it generates mixins amenable to TypeDoc

#

not whether it generates mixins more amenable to Typescript if that makes sense

#

BTW do you actually use typedoc-plugin-missing-exports? I see it in your package.json

#

but it's not in your typedoc.json, it should be as "plugin": ["typedoc-plugin-missing-exports"]

#

I think maybe this plugin is intended to help with some of these import(...) issues we were having?

vast horizon
rigid zinc
#

Yeah okay that's annoying, too bad that it doesn't work

#

I wanted to test it out myself

#

but I don't have the whole build pipeline atm, it seems to depend on a gulpfile

vast horizon
#

disclaimer, this was about a year ago that I tried it

rigid zinc
#

at least the import(...) resolution works

verbal quartz
#

If your project references classes which are built into the language (e.g. HTMLElement), this package will result in those types being documented to. If you want to prevent this, set TypeDoc's excludeExternals option to true. The default pattern for determining if a symbol is external will exclude everything within node_modules.
from the package description

#

given that foundry might want to include types for stuff like prosemirror or socket.io this may not be ideal

rigid zinc
#

Update, it ended up taking me a bit but I got mixins working! DM'd Atropos my work but if anyone wants to see my solution I'd be happy to share.

#

Here's how it ends up looking

#

(You can tell it involves a mixin because it says it inherits from EventEmitterMixin(Object).addEventListener which is the mixin at work)

#

(before methods like this simply wouldn't show up)

vast horizon
#

Spectacular work @rigid zinc.

#

It occurs to me that when airing my grievances with TSDoc earlier I actually forgot the worst one, so I have another to put on your hit list, if you're willing to continue saving the day.

rigid zinc
#

sure! I might have to get to it later

#

but I'll roll for initiative

vast horizon
#

not urgent, it's simple to reproduce

#
export class Foo {

  /**
   * Given an input number, return the lucky version.
   * @param {number} n    Some input number
   * @returns {number}    The lucky number
   */
  getLuckyNumber(n) {
    return n / 2;
  }
}

export class Bar extends Foo {

  /** @inheritDoc */
  getLuckyNumber(n) {
    return n * 2;
  }
}
#

Generated types:

export class Foo {
    /**
     * Given an input number, return the lucky version.
     * @param {number} n    Some input number
     * @returns {number}    The lucky number
     */
    getLuckyNumber(n: number): number;
}
export class Bar extends Foo {
    /** @inheritDoc */
    getLuckyNumber(n: any): number;
}
#

note that the documented type of param n is now any.

rigid zinc
#

Yeah that's obnoxious

vast horizon
#

This is the main reason we have so much any in our docs.

rigid zinc
#

how does it display on the documentation website?

vast horizon
#

because we use @inheritDoc a lot.

vast horizon
rigid zinc
#

Okay yeah that's really annoying

#

Dumb question first

#

How does it fare with {@inheritDoc}?

#

Or {@inheritDoc Foo.getLuckyNumber}

vast horizon
#

none of these options have worked for me

rigid zinc
#

Alright, I'll take a jab myself then, it's technically a tag that can take an argument is why I was asking but a bare @inheritDoc should've worked

vast horizon
#

yeah, tough because we rely on that tag extensively.

rigid zinc
#

First observations, the documented example works fine

vast horizon
#

@override doesn't work either.

rigid zinc
#

yeah rough

rigid zinc
#

Weird, it was closed as completed

#

As of May 9, 2022

vast horizon
#

the issue is perhaps incorrectly limited in scope to "static class members"?

#

but testing them as static methods still has an incorect type on the n param

rigid zinc
#

Digging into the PR it seems to assume the source files are Typescript

#

So it was there to support it in TSDoc + Typescript files

#

there's no implementation here for @inheritDoc + TSDoc + JS files

#

This is a legitimate feature request here it seems, I'll see if I can chip away at a PR to Typescript.

vast horizon
#

it seems pretty shocking that it has so much JS support but not basic support for inheritence

#

thanks for anything you can do - we have our hands full with... you know ... making VTT features.

rigid zinc
#

Are you sure fixing a upstream documentation generation issue isn't more important? 😉

#

thanks for all the VTT features!

tall pebble
#

Updated my demo repo w/ a full example of generating docs from bundled type declarations.
https://github.com/typhonjs-fvtt-scratch/esm-d-ts-demo

I also came up w/ a mixin solution, but currently favor using a TS interface for the mixin for documentation purposes. You can do it full JS as well and I think LukeAbby has a tip or two in that regard that I'm interested to review. Using a typedef put the mixed in function in "properties" instead of "methods" on my attempt.

Nonetheless a good exercise to learn how to document a mixin. Glad to discuss anything else on esm-d-ts if it ever makes sense. If you feel like you hit the limit of what JSDoc can do there are options.

This includes setup of ts-lib-docs TypeDoc plugins that link all modern web / JS symbols to API docs. You can see the latter here in the hosted docs where the TextureLoader.loadTexture method returns a Promise with HTMLDivElement: https://typhonjs-fvtt-scratch.github.io/esm-d-ts-demo/classes/TextureData.html#loadTexture

ts-lib-docs can be used with any TypeDoc pipeliine.

quaint nest
#

Now that it's the next morning for me and I'm not in a planned session of Balder's Gate 3, I can also contribute to this. 😀

A lot was already laid out here. The main recurring problem that I've seen which would be relatively easy to fix, is when polymorphism isn't adhered to. We can get around this, but at the cost of making the superclass more weakly typed. It's especially hard though when things inherit from built-in language constructs.

You guys are already talking about mixins, so I don't need to lay that out again.

And the last thing off the top of my head is DataModel, because that's been eating all the time I could spare on the types project. It's just absolutely complex for three reasons:
1: It's essentially modeling three states of the same data with a few option properties: constructor arguments, properties, serialized. It's basically the types of an ORM and schema validator in one.
2: The options could be designed in a way that would make them easier to type. Right now the types for a field start out kinda in the middle between really loose and really tightly typed. One option (nullable) pulls it in a more loose direction. Another (required) pulls it an more strictly typed direction.
It would have been much easier if fields start out as strictly one type and one can allow undefined/null via options.
A really good example how such an API can be designed is the zod validation library.

GitHub

TypeScript-first schema validation with static type inference

#

3: What's really confusing is that different predefined fields have different defaults for required and nullable, which are not obvious from the name. This is not only a pain to type them correctly, but they are also awkward to use. One couldn't even document this correctly with just a single @param tag, because this would only reflect the type in one of the above mentioned states of the data. I have had to resort to evaluating the result of a field in my head documenting it like this, just so I don't constantly have to go back and look at the source again when typing and documenting schemas.

GitHub

Unofficial type declarations for the Foundry Virtual Tabletop API - League-of-Foundry-Developers/foundry-vtt-types

#

I know a lot of this can't be changed anymore because it would be breaking. But it would be nice if it would be documented better.

#

Overall, thanks for taking the time for looking into this.

verbal quartz
quaint nest
vast horizon
#

Preview Client ESM Types - V2

Hey folks. Thanks to some excellent help by @rigid zinc yesterday I was able to identify and resolve some problems with the v1 types:

  1. Certain typedefs were not used properly because they were not imported/exported.
  2. Our mixin pattern was not detected in a way that added properties of the mixin class to the mixed class.

These improvements have resolved a number of issues in the generated types, specifically for the client-esm submodule. Similar improvements still need to be made in the commons submodule. Remaining pain points with types which we hope to continue working on are:

  1. The @inheritDoc directive is not working properly which incorrectly causes the params of subclass methods to be typed as any instead of as the type of the same parameter in the parent class.
  2. Typedefs which are delcared and used within the same file are not automatically exported.

This should look like an improvement on the preview posted yesterday. Again, please focus on the client-esm submodule for now but I'll be working on applying these improvements to the rest of commons gradually as we go.

verbal quartz
#

inheritdoc is wierd, because I know the TS language server can detect and handle it (I use it a lot at work for... reasons)

#

is the inheritdoc issue one with tsc or typedoc?

vast horizon
tall pebble
#

Indeed tsc when creating declarations from JS does not support @inheritDoc; also listed as unsupported on the Typescript JSDoc page. There was a long running issue on Typescript repo, but not resolved per se.

esm-d-ts handles #4 above already; all typedefs are promoted to the top level.

I'm looking into post processing bundled declarations generated to support @inheritDoc via ts-morph. I already planned on adding general post processing support options for custom post processing by the dev / user; this would just be a built-in post processing stage.

Basically I'm just trying to create a killer tool for ESM developers generating type declarations, so this is a very nice feature to have as it is otherwise unsupported out of the box w/ tsc.

vast horizon
#

Feels like a glaring omission in typescript.

#

given that TS ostensibly supports JS sources and a subset of JS documentation tags which allow it to be inferred as TS.

tall pebble
#

I do like that they are engaging the JS world / JSDoc world though; it's tremendously powerful to use TS types and mechanisms in JSDoc itself; granted once you do that you need to use doc tools like TypeDoc / TS related doc tools to generate docs. IDE support is pretty good and getting better all the time.

vast horizon
#

Honestly the IDE support is the most important thing - for our team - we’d like to make it easy for community devs to enjoy the same advantages we do internally in the IDE

tall pebble
#

Absolutely. I'm a WebStorm user and while it's easy to get nice intellisense support for the Foundry client code having type declarations will help the VSCode camp out a bunch for easier / better setup.

vast horizon
#

the inheritdoc thing is a real sticking point because I think you can understand that we are reluctant (unwilling) to repeat docstrings when the behavior of the subclass is aligned to that of its parent

tall pebble
#

The trick or at least another wrinkle in IDE support is creating source maps for the types. If the IDE is configured w/ just the types clicking through will bring you to the declarations and not the source. It is nice to jump to the client source as well. Jumping to the source occurs in WebStorm when you link the client source code as a library; presumably on the VSCode side when folks symlink to it in their project repo. I haven't spent too much time in VSC.

tall pebble
#

Alright... Long coding session. I have working postprocessing and have initial support for @inheritDoc. Granted all of this is experimental as while my test cases are a reasonable start they aren't a massive codebase. I have large code bases just not any that rely heavily on inheritance where @inheritDoc would come into play as I favor composition instead in general.

Updated the demo repo using the next beta of esm-d-ts. There is a mainline and postprocessed version of the docs. Check the classes Base through PartialInherited2_A to see that types are filled in from the parent where applicable. The generated doc examples can be found here.

No post processing:
https://typhonjs-fvtt-scratch.github.io/esm-d-ts-demo/main/

With post processing (@inheritDoc support):
https://typhonjs-fvtt-scratch.github.io/esm-d-ts-demo/post/

You can compare the type declarations here:
https://github.com/typhonjs-fvtt-scratch/esm-d-ts-demo/tree/main/.types

Edit: A beta release is available now on NPM; for a dev dependency: "@typhonjs-build-test/esm-d-ts": "^0.2.0-next.1"

vast horizon
#

Hey @rigid zinc (or others) I've got a few questions if you are able to share some TS wisdom with me that could help resolve some fairly commonly occurring problems.

  1. Is there some way that we can declare certain namespaces as existing so that tsc will not balk at something like game.i18n.localize(). Obviously a goal would be for eventually everything to be documented/understood, but it's going to be a long time until we get there and so in the short term it would be good to have a way to say something like "don't bother checking game.*". Is such a thing possible?

  2. I've encountered a problem with your mixin solution. Unfortunately within a resulting mixed class typescript does not understand this.constructor and thinks it is Function resulting in errors like:

client-esm/applications/api/application-v2.mjs(223,41): error TS2339: Property 'inheritanceChain' does not exist on type 'Function'.
  1. We add some additional functions to certain namespaces like Math. Similar question to 1 I suppose, but how can I instruct Typescript to either understand or ignore functions like Math.clamp?

  2. Having some trouble with core JS DOM/Event APIs for example:

element.querySelector(".some-selector").innerText;  
// TS2339: Property  innerText  does not exist on type  Element 

OR

const target = event.target;
const actionButton = target.closest("[data-action]");
// TS2339: Property  closest  does not exist on type  EventTarget 

These are both valid JS uses, but do I have to more aggressively document them?

#
  1. How to document a special class of Map which always contains a particular type of record, i.e.
class MyMap extends Map {}

Where instances of MyMap always contains values of MyType, i.e. MyMap<string, MyType>

rigid zinc
#

Hey!

tall pebble
#

I can take a stab at #1. You can create a globals-fvtt.d.ts like:

declare global {
   export type ApplicationHeaderButton = object;
   export type CompendiumCollection = object;
   export type DocumentCollection = object;
   export type EditorView = object;
   export type Folder = object;
   export type FontFamilyDefinition = object;
   export type game = object;
   export type ProseMirrorCommand = object;
   export type RollTable = object;
   export type Schema = object;

   export namespace foundry {
      namespace abstract {
         export type Document = object;
      }
   }
}

export {};

Then associate it in your tsconfig.json in include. That happens to be the one I use for generating docs on my side from the globals accessible in my codebase that make it through the declarations, but this should function similarly. Same thing likely possible w/ #3.

rigid zinc
#

Sorry I took a moment to reply

vast horizon
rigid zinc
#

I mean it's not like this is my job :P

#

so hour doesn't really matter

#

anyways, as far as Math.clamp goes it should probably be built in by now

#

you probably don't have an updated enough tsconfig

vast horizon
rigid zinc
#

Ahhh nvm then

#

Okay yeah

vast horizon
#

we add it to the Math namespace.

rigid zinc
#

Do you want to add it in JSDoc or would you mind a .d.ts file?

#

You'd do it like this in a .d.ts file:

declare global {
    interface Math {
        clamp(x: number, y: number, z: number): number;
    }
}
#

If it whines:

Augmentations for the global scope can only be directly nested in external modules or ambient module declarations.(2669)
Add export {}; as the first line

rigid zinc
#

I'll need to actually run this to check so give me a moment

vast horizon
rigid zinc
#

Yeah I'll look if there's a JSDoc way

#

Can you send the source (with JSDoc) of the Math.clamp function as it is today?

#

well if anything's changed recently at least

#

I can pull up like v11's source and look myself if nothing's changed

vast horizon
#

it's basically:

/**
 * Bound a number between some minimum and maximum value, inclusively.
 * @param {number} num    The current value
 * @param {number} min    The minimum allowed value
 * @param {number} max    The maximum allowed value
 * @return {number}       The clamped number
 */
export function clamp(num, min, max) {
  return Math.min(Math.max(num, min), max);
}

Object.defineProperties(Math, {
  clamp: {
    value: clamp,
    configurable: true,
    writable: true
  }
});
#

I don't think we have access to @memberof to do @memberof Math

rigid zinc
#

@namespace may work

rigid zinc
rigid zinc
#

Generates this for me:

/**
 * @extends {Map<string, MyType>}
 */
export class MyMap extends Map<string, MyType> {
    constructor();
    constructor(entries?: readonly (readonly [string, MyType])[]);
    constructor();
    constructor(iterable?: Iterable<readonly [string, MyType]>);
}
#

The constructors probably seem a bit confusing but it's the constructors of Map<string, MyType>

#

Can you see if it works in your code?

#

On #4 the issue is that querySelector returns Element which unfortunately can include elements that don't have innerText as a valid property.

vast horizon
#

we can't do "inline casting" in JS

#

but I guess I can do:

/** @type HTMLElement */
const title = element.querySelector(".window-title");
title.innerText = "foo";
#

bit annoying to have to do that.

rigid zinc
#

You can do inline casting

#

technically

#
/** @type {HTMLElement} */ (document.querySelector(".some-selector")).innerText
#

I dislike the syntax

vast horizon
#

barf

rigid zinc
#

I'll show you how to make querySelector always return a HTMLElement as soon as I figure out how JSDoc does declaration merging

#

It's technically more wrong but it shouldn't matter

#

because I doubt you're querying SVGs and the like

vast horizon
#

the .closest() case doesn't work with that solution though, because if you do:

    /** @type HTMLElement */
    const target = event.target;
    target.closest("[data-action]"); 

Results in

TS2740: Type  EventTarget  is missing the following properties from type  HTMLElement :
accessKey, accessKeyLabel, autocapitalize, dir
, and  282  more.
#

so hard to understand why people voluntarily choose to work in this environment 🤣

rigid zinc
#

Well when I work in TS I pull in a library if I'm heavily using css selectors

#

that'll automatically type it correctly based upon the actual element type

#

it's pretty neat

#

I avoid working with raw css selectors most of the time though

#

it's not a problem in say, Svelte, React, etc.

#

this is a common pain point though, I agree

rigid zinc
#

Ah

#

you're missing {s

#
    /** @type {HTMLElement} */
    const target = event.target;
    target.closest("[data-action]"); 
#

so this was actually just invalid JSDoc

vast horizon
#

not so

rigid zinc
#

So technically tsc is right here

#

you can add event listeners to non-HTMLElements

#
    const target = /** @type {HTMLElement} */ (event.target);
    target.closest("[data-action]");
#

This will work

#

Why? Because they desugar differently. This was JSDoc's design decision btw

#

not TS's

#
    /** @type {HTMLElement} */
    const target = event.target;

Transforms to

    const target: HTMLElement = event.target;

While

    const target = /** @type {HTMLElement} */ (event.target);

Transforms to

    const target = event.target as HTMLElement;
#

The first is saying "Its type is HTMLElement" and then Typescript comes back like "well it's missing all this data from HTMLElement"
And the second one is saying "Make its type HTMLElement (cast)" and so Typescript just accepts it

vast horizon
#

unfortunate. I think that inline (interspersed with actual code) documentation is a line we probably won't cross

#

but there's bound to be some impurities

rigid zinc
#

Yeah honestly I'm surprised you're type checking function bodies

#

since if you're not using Typescript its bound to be annoying a fair portion of the time

#

it just doesn't have enough info to determine if it's safe or not

#

You could document the type of event.target to be HTMLElement globally though

vast horizon
#

Yeah honestly I'm surprised you're type checking function bodies
Is that something that can be disabled?!?

#

guide me!

#

I'm just using checkJs: true

rigid zinc
#

Well how much of checkJs do you like? Just the intellisense?

vast horizon
#

Regarding question 2 in #1174045212679606412 message, it looks like associating this with the mixin solution was a false lead, because I'm having the same problem in a very basic (non-mixed) class where using:

this.constructor.preloadSound(src);

Is giving:

TS2339: Property  preloadSound  does not exist on type  Function
rigid zinc
#

Yeah I kinda thought this was the classic "class prototype = function prototype" issue

vast horizon
rigid zinc
#

ahhh

vast horizon
#

I definitely care more about function signatures than function bodies

rigid zinc
#

Here's my first suggestion then, the easiest way to tidy up and improve the type declaration, I think, would be to run tsc on the generated .d.ts files

#

Now you'll lose some of what checkJs tells you about generation, if something just isn't getting generated they'll be no errors

#

but checking for errors in the .d.ts files you generate will immediately lead you to some errors

vast horizon
#

okay, so I take it from that advice that there is not an easy way to turn off checking within function bodies?

#

you immediately built my hope up 😛

rigid zinc
#

haha, I know there's some way but I'm actually learning JSDoc as I go

#

I'm looking up the TS concepts and finding JSDoc equivalents

#

there's definitely tools that validate just JSDoc though is what I mean

#

but I'm not super experienced in JSDoc, just experienced in TS

tall pebble
rigid zinc
#

It's hard to prove a negative

#

but I think that TSDoc may not have a supported way to write global declarations like Math.clamp

#

You don't have to re-type it though

#

I put this in a file named global.d.ts

import { clamp } from "...";

declare global {
  interface Math {
    clamp: typeof clamp;
  }
}
#

And then I got this documentation, where before I added the file it just errored

#

Is the worry more intellisense of Math.clamp in the repo or documenting Math.clamp in a way that TSDoc documents though?

#

If you need TSDoc to document it, make it a global.ts file.

vast horizon
#

yeah... I suppose that's not so bad. Do you know what this sourceType: "module" is about?

rigid zinc
#

ah make it mts

#

I forgot you don't have type: module in your package.json or something

vast horizon
#

aah yeah that did it

rigid zinc
#

If you need it to get documented, you'll probably have to make it global.mts (the name doesn't actually matter, the difference is between .d.mts and .mts)

#

This also happens to solve the whole "game" problem btw

#
import { clamp } from "...";

declare global {
  interface Math {
    clamp: typeof clamp;
  }

  const game: any;
}

If you did this

vast horizon
#

this is where the bodies will be buried

rigid zinc
#

haha

#

you can also import game

#

and do typeof game

#

it probably won't be complete

#

but it'll type it based upon its best inference

#

ah on second thought it'll probably type it as {} as it starts out empty

#

Lol

#

not too helpful

#

Ah! As promised btw

#

Add this inside declare global:

  interface ParentNode {
    querySelector(selector: string): HTMLElement | null;
  }
#

Actually nvm these definitely should go in a .d.mts file

#

I suggest adding a global.d.mts so it won't get documented (alongside the global.mts which gets documented)

export {};

declare global {
  interface ParentNode {
    querySelector(selector: string): NodeListOf<HTMLElement> | null;
    querySelectorAll(selector: string): HTMLElement | null;
  }
}

This'll 'fix' the typing of them. But actually the typing as it is right now just reveals the messy truth that you can get non-HTMLElements

rigid zinc
# vast horizon Hey <@133642107259846657> (or others) I've got a few questions if you are able t...

And I think that "answers" everything? In short:

  1. Use the global.mts file. Or global.d.mts if you don't want these extensions documented.
  2. This is a normal problem for this.constructor because you could technically get any Function (because classes were just functions before)
  3. Same as 1
  4. Sorta same as 1 but use a global.d.mts for sure
  5. Use @extends {Map<string, MyType>}

I can elaborate if some of them weren't answered completely!

vast horizon
#

thank you, really appreciate it

#

this.constructor is going to be a big problem

#

but I knew that before from an earlier exploration of TS

rigid zinc
#

Ah are you trying to remove all type checking errors?

tall pebble
#

Yep, just a checkJs problem per se. Shouldn't affect the declaration generation.

rigid zinc
#

Also I got started on work to add better @inheritDoc support to tsc

vast horizon
#

im not sure what my goals are really

vast horizon
#

I'm probably wasting time

tall pebble
#

I appreciate you taking all this effort.

rigid zinc
#

This is the main reason why they don't just make T.constructor = T like you may expect

#

that is, a bunch of load-bearing JS code expects the code they show to work

#

probably isn't any solace, I just personally appreciate knowing why these rough edges exist

vast horizon
#

I think I just need to establish a better approach for catching and eliminating errors in generated declarations/documenation without worrying about anything in the actual code body

#

so your suggestion about building the declarations and then checking them somehow might be the workflow I need to learn

rigid zinc
#

Sure! Add a _dts/tsconfig.json:

{
  "include": ["**/*.d.mts", "**/*.d.ts"],
  "compilerOptions": {
    "lib": ["DOM", "ES2023"]
  }
}
#

Then your workflow would be:

  1. Run npx tsc at the root of the project to generate declarations.
  2. cd _dts
  3. Run npx tsc to type check the declarations
  4. Build the docs and check them out too probably
#

as long as you have added the _dts/tsconfig.json you should also be able to open the files in your editor and see issues

#

Lots of small issues like:

common/utils/http.d.mts:12:72 - error TS2339: Property 'onTimeout' does not exist on type 'Number'.

12 export function fetchWithTimeout(url: string, data?: any, { timeoutMs, onTimeout }?: number | null): Promise<Response>;
                                                                          ~~~~~~~~~

common/utils/http.d.mts:21:90 - error TS2304: Cannot find name 'int'.

21 export function fetchJsonWithTimeout(url: string, data?: any, { timeoutMs, onTimeout }?: int): Promise<any>;
#

bear in mind anything this reports SHOULD be an error in the documentation too

#

Here's an update to your tsconfig btw: "lib": ["DOM", "ES2023"], otherwise stuff like WeakRef won't resolve

rigid zinc
#

it'll be under compilerOptions

#

This is going to sound sarcastic

#

but fortunately there's only 294 errors

#

ah, actually that's only in two packages

vast horizon
#

I think the vast majority of them are just unrecognized global references from foundry.js

rigid zinc
#

yeah exactly

#

If you want to filter that out you can just copy all the errrors and then remove the ones that say TS2304

#

that kind of thing

#

Also for PIXI and such

#

This is actually important for documentation

#

npm install @types/jquery that kind of thing, there should be available types for everything somewhere, I can help you pull it in, it'll let you link to JQuery types etc.

vast horizon
#

clearing those there's 95 which seem meaningful

#

im only worried about ones in client-esm for now which is:

client-esm/audio/convolver.d.mts(37,14): error TS2416: Property 'disconnect' in type 'ConvolverEffect' is not assignable to the same property in base type 'ConvolverNode'.
  Type '(target: any) => void' is not assignable to type '{ (): void; (output: number): void; (destinationNode: AudioNode): void; (destinationNode: AudioNode, output: number): void; (destinationNode: AudioNode, output: number, input: number): void; (destinationParam: AudioParam): void; (destinationParam: AudioParam, output: number): void; }'.
    Target signature provides too few arguments. Expected 1 or more, but got 0.
client-esm/audio/convolver.d.mts(39,14): error TS2416: Property 'connect' in type 'ConvolverEffect' is not assignable to the same property in base type 'ConvolverNode'.
  Type '(destinationNode: any, output: any, input: any) => any' is not assignable to type '{ (destinationNode: AudioNode, output?: number, input?: number): AudioNode; (destinationParam: AudioParam, output?: number): void; }'.
    Target signature provides too few arguments. Expected 3 or more, but got 2.
#

it's not liking my ConvolverNode subclass

rigid zinc
#

Yup, you can solve the unrecognized global references in your global.mts if you'd like

#

let me take a look at that error

#

first gut instinct

#

it's probably not important

#

it's probably whining that you're making a subclass that isn't technically a subclass

#

because you give it an incompatible method implementation

#

which means that you can't use it in place of the superclass 100% of the time (because that method's signature is different)

vast horizon
#

I would disagree and say it is a valid subclass. The methods are

  /** @override */
  disconnect(...args) {
    this.#wetGain.disconnect();
    this.#dryGain.disconnect();
    return super.disconnect(...args);
  }

  /* -------------------------------------------- */

  /** @override */
  connect(destinationNode, ...args) {
    super.connect(this.#wetGain, ...args);
    this.#dryGain.connect(destinationNode);
    this.#wetGain.connect(destinationNode);
    return destinationNode;
  }

EDIT: now working

#

but I'm sure it's wrong and I'm wrong

#

for some reason

rigid zinc
#

Yeah so it looks like the issue is that target is optional in AudioNode

#

it also returns this in AudioNode

#

Fix #1:

  connect(node) {
    super.connect(this.#wetGain);
    this.#dryGain.connect(node);
    this.#wetGain.connect(node);

    return this;
  }
vast horizon
#

for that to work it needs to return the destinationNode

#

maybe I'm wrong

rigid zinc
#

I think I'm on an older version

#

where it's not returning anything at all

vast horizon
#

I've got the disconnect case solved though

rigid zinc
#

I see this implementation:

  /** @override */
  connect(node) {
    super.connect(this.#wetGain);
    this.#dryGain.connect(node);
    this.#wetGain.connect(node);
  }
#

Yeah you just needa make the parameter optional

rigid zinc
#

Yeah

vast horizon
#

no worries, I'm going to call it a night

rigid zinc
#

sounds good

#

I can help out typing connect correctly tomorrow if you'd like

#

good night

vast horizon
#

I solved it by changing explicit input and output to ...args

#

WebStorm/IntelliSense complains, but tsc is fine with it

rigid zinc
#

haha, you can also solve it like so, I think:

/**
 * @param [input]
 * @param [output]
 */
whateverMethod(input, output)
vast horizon
#

that requires re-documenting a method defined by a parent class

#

no thanks

rigid zinc
#

fair enough

#

Actually I think this will be solved when @inheritDoc actually works

quick tundra
#

I recently cruised through the core prototype injections to get their types updated if you'd like to purloin

rigid zinc
#

Do you mean in pf2e? Or what

quick tundra
#

yeah

rigid zinc
#

sure, I'll look at it at least

quick tundra
#

oh, I mean Atro

#

but anyone, naturally 😁

rigid zinc
#

ofc

rigid zinc
#

This of course

#

has also been done in foundry-vtt-types

quick tundra
#

but do you have the ascii art typed

rigid zinc
#

So you have two places to cross reference if you wish

#

I think we do actually LOL

vast horizon
quick tundra
#

it's not in the build!

vast horizon
#

Lol

quaint nest
#

We do have all of those globals typed, yes. Though without looking, I don't think we have the actual content in there, just string as a type.

verbal quartz
quick tundra
#

the ts dom lib is maybe a little too conservative over this: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget

Element, and its children, as well as Document and Window, are the most common event targets, but other objects can be event targets, too. For example IDBRequest, AudioNode, and AudioContext are also event targets.

verbal quartz
#

I mean it's true that element and it's children are the most common targets but not all children of element HAVE an innerText or a value property. Sure you could move all that up to Element or even make HTMLElement the default but then you're not modelling the DOM anymore as specified

quick tundra
#

a theoretical comfort with little practical value, and even then it could be a discriminated union

verbal quartz
#

best I can tell TS has always been about modelling how JS is specified, not how it is most commonly used. ¯_(ツ)_/¯

#

So realistically this is a JS issue but also more of a philosophical of what typesafety really means debate at this point.

vast horizon
#

If you want to take the philosophical high ground, the return type should depend on the type of this that querySelector was called on.

#

In my example, for instance, it is known that the “element” is an HTMLElement, so the return value of element.querySelector should be as well, I would think

#

But it’s not our choice to make

verbal quartz
#

I'd actually argue that your assumption is wrong. HTMLElement and SVGElement both inherit from Element. HTMLInputElement inherits from HTMLElement. Just from .window-title (so just a class) selector it's impossible to infer what the element is, so TS assumes the "lowest common demoninator", which is Element. However TS can infer some things automatically if you for example use ul then TS can infer that what you select is a HTMLUListElement.

That said, the list is hardcoded so unless I'm mistaken ul.my-list will confuse it again so that's only of limited help and definetly a shortcoming.

The point remains tho. Just from a class name or even a name attribute (or any other attribute) it's hard to determine what the resulting element is, so TS depends on you to tell it what you should expect from .window-title.

verbal quartz
#

the type of the element is known to you because you wrote and read the markup. TS doesn't do that. Everything that interacts with the markup requires additional input from the user because technically that's outside the language.

vast horizon
#

@verbal quartz I think you don't understand my comment properly.

#

if you have:

element.querySelector(someSelector)

and you know that element is a HTMLElement, you know the result of element.querySelector must also be an HTMLElement (or subclass thereof)

#

so I suppose my comment is that, ideally HTMLElement#querySelector would define its returned type as HTMLElement

#

While Element#querySelector would define its returned type as Element.

#

anyways, it's immaterial since its not in our hands either way

verbal quartz
#

Except it's not element.querySelector(selector) could be an SVG for example.

#

Sure saying it's an HTMLElement is a good guess but I'm not sure many developers like a good guess in their typesafe-ish language

verbal quartz
#

that said the return type of element#closestshould defiently be an HTMLElement (probably) 😄

quaint nest
#

As a side note, the generic param is the same thing as a cast, type-safety wise. I prefer to actually make it safe like so:

const element = document.querySelector(".foo");
if (!(element instanceof HTMLElement)) return;
// or
if (!(element instanceof HTMLElement)) throw new Error("'element' was not an HTMLElement.");

// After the check, TS treats `element` as an `HTMLElement`.

The throw version has the advantage, that you will catch errors in your selectors quite early. I basically got into the mindset, that from a script perspective, the DOM is untrusted input.

verbal quartz
#

oh yeah it defiently is 😄

quaint nest
#

Of course you can also replace HTMLElement with any other, more precise element class.

vast horizon
quaint nest
#

You could almost compare it to pattern matching in other languages.

verbal quartz
#

typeof also works

#
if(typeof value !== "number") return;
//TS now treats value as a number
quaint nest
#

How could I forget that one?! Yes of course. typeof for primitives, instanceof for "Classes", in for duck-typing.

low blade
#

Checking for null and undefined also work.

#

Orta.vscode-twoslash-queries is a great extension for VS Code users, takes the feature from the TS playground and puts it into VS Code. Just queries Typescript it's self for the answer of "What is the type of that thing I'm pointing at exactly?"

verbal quartz
#

How does that differ from the language server?

tall pebble
#

So I must say I really like this conversation as it led to some innovation in my tooling understanding problems that I didn't precisely face when I did the major overhaul of my ESM work to support complete type declarations; the bulk of which happened March-July this year. I'd like to better understand the nature of what exactly is causing heartache so to speak when running checkJs. The goal in general for the core team is to create public declarations of ESM code. checkJs is useful, but too aggressive considering it is checking everything. What if there was a way to filter checkJs output to only show errors with what precisely is represented in the public declarations. What I mean with this is checkJs ignoring the content of function / method bodies, but retaining all warnings / errors for what is actually in the public API / declarations. At least this is my inclination for a feature to develop in said tooling from this conversation. Would that be a useful feature in general (Atro and others)?

I feel like I need to preface this query with I don't expect anyone here to use said tooling. I'm just trying to create awesome tooling that eases the pain for all ESM developers approaching declaration generation via JSDoc.

low blade
# verbal quartz How does that differ from the language server?

It does not differ from the tooltips that the language server offers in content, just that you can tag what you want to debug in a comment and see many at once, which is useful when working on stuff that you want to check that spans a fair bit. If I was to replace string with number I could see it change all throughout the sample snippet at once, without needing to manually move the mouse over each one to check.

verbal quartz
#

aaaaah

#

cool

rigid zinc
#
const foo = document.querySelector("input");
foo?.value;
#

foo is typed as HTMLInputElement | null so this works completely fine.

#

Surprised to see that no one mentioned it

#

The issue is that when you do .some-class or #some-id there just isn't enough information to know if it's an HTMLElement or not.

#

This used to be in a library that I enjoyed and used but now it's 100% core

#

It's not even some compiler intrinsic (or if it is, it doesn't have to be, like I said I used it when it was just a library), you can genuinely do enough type level ops on a string to figure out what type querySelector should be.

#

So if you wanted to you could add it to your global.mts/global.d.mts and it just works

quick tundra
#

So that signature overload is nice to have the once or twice per decade it can be invoked

rigid zinc
#

Partially the core implementation is less useful than the library version

quick tundra
#

whoa, I bet those types are gnarly under the hood

#

that's pretty slick, though

rigid zinc
#

haha

#

yeah a buncha string ops

#

gotta be

quick tundra
#

confirmed eyebleach of screen-long literal unions and conditional types

vast horizon
#

Is there a conventional way to declare that a typedef Object may have additional keys and values beyond what is explicitly documented?

quick tundra
#
    type EmbeddedDocumentUpdateData = {
        _id: string;
        [key: string]: unknown;
    };
#

if it's just an object of unpredictable key/value pairs, something like this:

type DocumentFlags = Record<string, Record<string, unknown> | undefined>;
vast horizon
#

sorry, my questions are never about "how is this done in Typescript"

#

my questions are about: is there a way to document this in JS that Typescript understands

quick tundra
#

ohh using typedef syntax

#

my b

rigid zinc
#

I know how, give me a half hour to type it up though

#

not on ideal platform

vast horizon
#

i am not in a rush at all

rigid zinc
#
/**
 * @typedef {Record<string, unknown>} Foo
 * @property {number} bar
 */
#

That should do it

tall pebble
#

So you can also embed any TS type in @typedef:

/**
 * @typedef {({
 *   _id: string;
 *   [key: string]: unknown;
 * })} Foo
 */
rigid zinc
#

Yeah but documenting the property is what matters

tall pebble
#

Useful when it's necessary for something more complex. You can provide a description after Foo.

vast horizon
#

@rigid zinc thanks, any suggestion why the following typedefs are not producing an expected result?

/**
 * @typedef {Object} DatabaseOperation
 * @property {Document|null} [parent=null]      A parent Document within which certain Documents are embedded
 * @property {string|null} [pack=null]          A compendium collection ID which contains the Documents
 * @property {string} [parentUuid]              A parent Document UUID provided when the parent instance is unavailable
 * @property {boolean} [broadcast]              Whether the database operation is broadcast to other connected clients
 */

/**
 * @typedef {DatabaseOperation} DatabaseGetOperation
 * @property {Record<string, any>} query        A query object which identifies the set of Documents retrieved
 * @property {boolean} [index]                  Return indices only instead of full Document records
 * @property {string[]} [indexFields]           An array of field identifiers which should be indexed
 * @property {false} [broadcast]                Get requests are never broadcast
 */
#

Results in:

export type DatabaseOperation = {
    /**
     * A parent Document within which certain Documents are embedded
     */
    parent?: Document | null;
    /**
     * A compendium collection ID which contains the Documents
     */
    pack?: string | null;
    /**
     * A parent Document UUID provided when the parent instance is unavailable
     */
    parentUuid?: string;
    /**
     * Whether the database operation is broadcast to other connected clients
     */
    broadcast?: boolean;
};
export type DatabaseGetOperation = DatabaseOperation;
rigid zinc
#

Interesting, I think @property only works with object and Object specifically

#

let me take a look at fixing this

#

I can immediately solve this, it's just ugly

#
/**
 * @typedef {object} DatabaseGetOperationBase
 * @property {Record<string, any>} query        A query object which identifies the set of Documents retrieved
 * @property {boolean} [index]                  Return indices only instead of full Document records
 * @property {string[]} [indexFields]           An array of field identifiers which should be indexed
 * @property {false} [broadcast]                Get requests are never broadcast
 */

/** @typedef {DatabaseOperation & DatabaseGetOperationBase} DatabaseGetOperation */
#

This will generate the right type, just, again ugly

vast horizon
#

😭

#

thanks

#

I do not love it

#

but thanks

rigid zinc
#

if I find a more ideal solution I'll let you know

#

but at this point I'm asking in the JSDoc communities

#

I think the answer I'll probably get is "@property is defined to only work with object/Object not any arbitrary object"

rigid zinc
verbal quartz
rigid zinc
#

ah must have missed it

verbal quartz
#

No worries :D

verbal quartz
#

I wonder if you're starting to push on the boundaries of what TSC/JSDoc is capable of with vanilla JS

verbal quartz
#

I've looked at OpenLayers again, since it seems to be a good analogy in terms of complexity and extent of the documentation, and their docs site to also be generated using jsdoc, with the help of a ferw plugins

"jsdoc": "4.0.2",
"jsdoc-plugin-intersection": "^1.0.4",
"jsdoc-plugin-typescript": "^2.2.0",

That and they claim the packagecontains auto-generated .d.ts files

#

but yeah, "ugly" typedefs are likely the only solution and you will have to bite the sour apple. JS isn't really made for this as far as I can tell.

#

ah @typedef isn't even part of the JSDoc dictionary, it's part of google's Closure compiler, but JSDoc can work with it.

vast horizon
vast horizon
verbal quartz
#

I know closure also defines it in their documentation as a tag. JSDoc supports "dictionaries" in their config and closure is among those as a default even so I can image they are documenting it on their page for that reason

#

of course it's also entirely possible that this is a case of convergent evolution

verbal quartz
vast horizon
#

So while documenting JS is not the “primary function”, it is the assumed responsibility by TSDoc, until or unless some other better tool comes along and takes its place

#

We are just trying to swim with the current and deliver the best docs we can.

verbal quartz
#

My understanding was that it's less because TSDoc said they got this (they don't got this), but because more and more people switched over to typescript which makes large swathes of JSDoc simply obsolete. That said, JSDoc does support plugins and from what I can tell that scene is still alive and active

#

so what JSDoc doesn't do natively could be solved by a plugin

#

now where have I seen this before (modules)

vast horizon
#

No

#

Your understanding is incomplete

verbal quartz
#

I see

#

I mean TSDoc can't really take over the job of JSDoc because it's designed to work for a different language (for all intents and purposes)

verbal quartz
#

looking at the repos both projects are dead. fun

rigid zinc
#

Wait JSDoc is discontinued? I've never seen anything about that on the repo or site or anything huh

#

It has a push as of yesterday too and it looks like its additions per week graph is relatively high still

#

if you mean JSDoc is missing core features for actually generating a real documentation site and the like then I'd be inclined to agree, I just thought development was still ongoing and all

quick tundra
verbal quartz
wide hollow
#

So, I've been hammering away at getting the League project up to date for v11, and I've run into a bit of a problem.

  • Token extends PlaceableObject
  • Token.vision is of type VisionSource
  • VisionSource.fov returns type PointSourcePolygon|PIXI.Polygon
  • BUT PlaceableObject.vision.fov is typed to PIXI.Circle, which has additional properties beyond the guarantee provided by the PIXI.Polygon type of Token.vision.fov
#

I went through and resolved a ton of stuff to determine that this is a core foundry issue. Maybe this is just a documentation problem and actually a token's vision is otherwise guaranteed to be a PIXI.Circle?

#

I brought this up here since this is part of the general "improving typechecking" that's happening, it's possible this should get separately filed in #1045406090285826158

vast horizon
#

Not at my pc now so cannot say with absolute confidence

#

There may be some exception cases

wide hollow
#

When you say evolved, you mean in the upcoming v12 release?

(No rush on resolving this, American holiday tomorrow. But wanted to flag for when you or another core dev has time to look)

vast horizon
#

No I mean since earlier versions pre v11

wide hollow
#

Ah, so just an outdated type annotation in the base PlaceableObject class?

vast horizon
#

I believe so. I’ll confirm for you a bit later (hopefully I remember)

wide hollow
#

👍 Thank you!

vast horizon
#

I actually think the true issue is that PlaceableObject#vision should not exist at all as a property. It's not used and is incorrectly typed, as you have pointed out.

wide hollow
#

👍 Glad I was able to flag down the issue

verbal quartz
#

yay, improvements

verbal quartz
#

Is it correct that foundry still uses TypeDoc to create the actual https://foundryvtt.com/api page?
Allegedly TypeDoc does recognize the @inheritDoc tag. I'm not entirely sure about the pipeline that foundry uses to generate the page. Do you go js -> typedoc or js -> d.ts -> typedoc?

#

well the question might be moot since typedoc uses tsc under the hood anyway, so it seems

tall pebble
#

Re: TypeDoc and @inheritDoc. It supports moving comments from parent declarations and not types for the method / function parameters. It won't consider, "oh hey this is a Javascript project lets correct the '@inheritDoc' issues w/ the TS compiler". Of note I suppose is that it does not look for @inheritdoc, so you need to use @inheritDoc. The code is here to verify: https://github.com/TypeStrong/typedoc/blob/master/src/lib/converter/plugins/InheritDocPlugin.ts

I do a lot with TypeDoc. My "default modern theme" which is more like an augmentation of the default theme directly led to some massive performance improvements in TypeDoc 0.25.1 / credited on that release for the ideas; dynamic left side navigation component and compressing the search index. I'm updating my "default modern theme" presently to take advantage of the navigation index that is now generated. I use Svelte (of course) to display the left side navigation in my theme augmentation.

tall pebble
#

The pipeline you brought up though "js -> d.ts -> typedoc" is how my documentation effort / pipeline works and it is suitable for public API documentation. IE no private methods / declarations that would be useful for internal API docs not publicly distributed as those are stripped from the type declarations. This pipeline allows one to correct for inefficiencies with tsc for JS projects and do other handy things. One of those inefficiencies brought up prior by Atro is that tsc doesn't handle static private declarations in JS -> .d.ts well; you need to correct for this in generating declarations.

Outside of recent @inheritDoc support in my tooling efforts an example of a handy passthrough in type declaration generation is preserving JSDoc comment blocks with @module / @packageDocumentation which are normally stripped by tsc in creating declarations. These tags are meaningful for TypeDoc / documentation purposes.

verbal quartz
#

Right. While cool, this doesn't answer my question. What I primarily wanted to figure out is if the lack of a support for inheritdoc tags may not be as big as previously thought because so far we've only really looked at what tsc does and the resulting declarations. From what I can see at the top of the thread, that is in fact not the main goal. Fact is typedoc does extra processing for certain tags so the declaration files not containing the extra docs is not a huge issue since they seem to be a happy sideproduct at best

#

it's also entirely possible I have completely misunderstood the goal of this venture or maybe it has shifted slightly

tall pebble
#

Two pronged from what I picked up from the conversation. A general desire for core generated declarations to correctly have typed method / function parameters via @inheritDoc usage and bonus that this gets into generated documentation. I think it's great that a lot of this work by the core team is hitting the first aspect in an attempt to produce more accurate default declarations.

verbal quartz
#

Yeah I read it differently. ^^
My reading was that the primary concern is the improvment of the api page. One step on the journey to that is generating declarations to see how the documentation comments need to be changed so typedoc can generate better docs.

#

So I guess may point is, if at least some part of @inheritDoc does work in the final end product (the generated page) then maybe the lack of native support for this tag is not a huge issue, if an issue at all

#

especially since it doesn't seem that foundry wants to publish the types themselves anyway. In atropos own words

It is my hope that these .d.ts (or .d.mts for ESModules) files can be a strong starting place for the community Typescript types which would further refine them to improve their accuracy and usefulness beyond what is auto-generated.

#

which sorta reminds me that a complete adherence to @inheritDoc may not be advantagous anyway since foundry does occasionally break polymorphism so inherited docs would be wrong in those places anyway, unless typedoc can merge the blocks (I haven't checked that yet)

#

naturally please correct me if I am wrong ^^

tall pebble
#

Re: the general desire for more accurate declarations is "pain point" #3 here: <#1174045212679606412 message>

Alas so how TypeDoc works... It doesn't merge additional comments in @inheritDoc tagged symbols. It will produce a warning that additional comments will be overwritten. It will find the nearest parent with a doc comment that doesn't use @inheritDoc and copy that in entirety. And this is just the comment blocks and nothing to do with @param or types of the parent.

/**
 * Some more doc stuff that is ignored.
 * @inheritDoc
 */
someMethod(foo, bar) {}
verbal quartz
#

seems like a decent idea for a plugin or PR to typedoc then

verbal quartz
#

so it's part-of-jsdoc-but-not-really (but that's just arguing semantics)

vast horizon
#

The declaration files correctly have inheritDoc tags

#

But the built docs ignore that and instead document as “any” type params

verbal quartz
#

I am fully aware. I think an important distinction to make here is that JSDoc is a definition and a generator, while TSDoc is technically only a proposal. I know that difference doesn't help anybody at this stage but it's probably still good to keep in mind

#

TSDoc doesn't offer any document generation (though it really should, like JSDoc)

quaint nest
verbal quartz
#

While that is true it seems like TSC is not doing that for some reason

#

Or maybe I am misunderstanding the mechanics

quaint nest
verbal quartz
#

Oh right, derp

quaint nest
#

I bet if you look in the generated .d.ts files and look at the overrides, those will likely already have nothing or any as types.

#

But even in typescript, as with most other type-safe langs, you have to write out the signatures. I only know one language that's type safe and has type inference where you might not have to do that: OCaml.

verbal quartz
#

I'd have to check my old Java code but I think you are correct

verbal quartz
#

what's funny to me is that vscode doesn't even recognize @inheritDoc as a valid tag, but it does with @inheritdoc in JS

#

but yes @quaint nest is absolutely correct. I built these classes

/**
 * @typedef {number} ClassOptions
 */

/** Some fancy class */
export class Foo {
  /**
   * Lorem Ipsum
   * @param {object} value
   * @param {ClassOptions} options
   * @returns {object} The initialized schema field
   */
  initialize(value, options) {
    return value;
  }
}

export class Bar extends Foo {
  /** @inheritDoc */
  initialize(value, options) {
    return value;
  }
}

export class Baz extends Foo {
  /**
   * Dolor est
   * @override
   * @param {object} value
   * @param {ClassOptions} options
   * @returns {object} The initialized schema field
   */
  initialize(value, options) {
    return value;
  }
}

which resulted in the following output

/**
 * @typedef {number} ClassOptions
 */
/** Some fancy class */
export class Foo {
    /**
     * Lorem Ipsum
     * @param {object} value
     * @param {ClassOptions} options
     * @returns {object} The initialized schema field
     */
    initialize(value: object, options: ClassOptions): object;
}
export class Bar extends Foo {
    /** @inheritDoc */
    initialize(value: any, options: any): any;
}
export class Baz extends Foo {
}
export type ClassOptions = number;
#

though what's bizzare is that tsc seems to have completely swalloed Baz's function.

quaint nest
verbal quartz
#

well the comment block is different but other than that it has the exact same signature

#

well if I rewrite the same classes in TS it works

#

classes.mts```ts
export type ClassOptions = object;
/** Some fancy class /
export class Foo {
/
*

  • Lorem Ipsum
  • @param value
  • @param options
  • @returns Some processed value
    */
    doStuff(value: object, options: ClassOptions): object {
    return value;
    }
    }

export class Bar extends Foo {
/** @inheritDoc */
override doStuff(value, options) {
return value;
}
}

export class Baz extends Foo {
/**

  • Dolor est
  • @param value
  • @param options
  • @returns Some processed value
    */
    override doStuff(value: object, options: ClassOptions): object {
    return value;
    }
    }
classes.d.mts```ts
export type ClassOptions = object;
/** Some fancy class */
export declare class Foo {
    /**
     * Lorem Ipsum
     * @param  value
     * @param options
     * @returns Some processed value
     */
    doStuff(value: object, options: ClassOptions): object;
}
export declare class Bar extends Foo {
    /** @inheritDoc */
    doStuff(value: any, options: any): any;
}
export declare class Baz extends Foo {
    /**
     * Dolor est
     * @param value
     * @param options
     * @returns Some processed value
     */
    doStuff(value: object, options: ClassOptions): object;
}
#

well mostly, but it illustrates f1r3's point

rigid zinc
#

I've looked into the TSC source n in order to maybe add signature inheritance. This is the least important part but TSC does look for @inheritdoc.
And it has some test cases explicitly showing the lack of signature inheritance

rigid zinc
verbal quartz
#

yeah I figured as much

rigid zinc
#

It assumes the classes are actually sound

verbal quartz
#

anyway, luke is going deep 😄

rigid zinc
#

But js class extensions are unsound a LOT of the time

#

Oh I was already looking because I was gonna add this as a feature

verbal quartz
#

so typescript has a check for @inheritdoc but doesn't process it?

rigid zinc
#

Well as was said earlier it processes the non-type documentation portions

verbal quartz
#

fair

rigid zinc
#

I brought up that it processes @inheritdoc solely because it was said only @inheritDoc was processed

verbal quartz
#

I wonder how much the sdoc and typescript people communicated or of the two projects are entirely seperate

rigid zinc
#

They're obviously not completely separate

#

because TSDoc came later and took a bunch of tags from JSDoc

#

but as for how much the projects actually talked?

#

no clue

verbal quartz
#

seperate as in "those two teams never coordinated"

#

ah yeah

#

I am willing to bet that the two teams/projects didn't really work together because TS natively supports some tags from the JSDoc/Closure/TSDoc set, (like @remarks or @defaultValue) but not others

#

so I suspect the TS team took what they thought would be useful only

rigid zinc
#

I mean the name betrays the purpose as far as TypeScript seems to have concerned itself

#

It's not going to put in a tag you can do in pure TS

#

@param working with types has two purposes

verbal quartz
#

but stuff like missing @inheritdoc does seem a bit wierd I guess.

rigid zinc
#

Not trying to be pedantic but it's not technically missing

verbal quartz
rigid zinc
#

it's just not inheriting function signatures

#

the guy who added inheritDoc

verbal quartz
#

fair enough

rigid zinc
#

bc it was one guy

#

probably didn't care about JS

#

and so never ran into that problem

verbal quartz
#

seems logical

rigid zinc
#

as far as I can tell there's been about 5 people who ever touched the inheritDoc code

#

I could easily be making up a false history as for why they never added support

verbal quartz
#

all we can really do is speculate

rigid zinc
#

technically I could just ask one of the implementors

#

I've seen them around but it's not important enough imo

verbal quartz
#

well it's clearly important enough for Atropos, for good reason 😄

rigid zinc
#

nono

verbal quartz
#

?

rigid zinc
#

asking about the history of @inheritDoc

verbal quartz
#

oh!

rigid zinc
#

not asking about function parameter inheritance

verbal quartz
#

gotcha

#

well asking about function parameter inheritance would probably be worthwhile tho

rigid zinc
#

yeah I've technically figured out how to add support already

verbal quartz
#

:O

#

make a pr!

rigid zinc
#

emphasis on the technically, it's not in a PR-able state

#

I'd be able to iterate and make it better faster

#

if it weren't for the fact that my language server crashes every other minute for some reason

#

it's not a computer spec thing for sure

#

because my CPU/memory/etc. are all fine

#

and I've looked and some other TypeScript contributors have lower specs and work faster

verbal quartz
#

heh, I looked at the code of Openlayers and they seem to use @inheritDoc once in the entire codebase

#

and it's all JS

rigid zinc
#

Yeah it might not feel obscure for Foundry

#

but @inheritDoc is a surprisingly rare tag

verbal quartz
#

it makes sense in my head but I am also used to ts just doing most of that out of the box

#

granted, 80% of my TS experience comes from workin in and with frameworks built to use TS, like Angular and NestJS.

hot lotus
wide hollow
#

Docs question: is there a reason most PIXI classes are still namespaced by PIXI, but PIXI.Point is typed as just Point

wide hollow
#

OK I see it in common/types.mjs

verbal quartz
#

Any news on this front?

vast horizon
verbal quartz
#

generally. I figured maybe you had gotten around to finding more things that require clarification in the meantime.

#

or stumbled upon more things is probably the more apt phrasing

vast horizon
#

No, not really. Just going to be a gradual and steady process of trying to improve things as we move them over to client ESM or otherwise revise them

verbal quartz
#

fair enough. Good luck then!

wide hollow
#

Hi, back to causing trouble by asking questions about types

#

In client/pixi/WebGL/shaders/base.js, BaseShaderMixin defines ShaderClass as a parameter that's typeof PIXI.Shader

#

But later class AbstractBaseFilter extends BaseShaderMixin(PIXI.Filter), and now that I'm typing things it's quite unhappy with the constructor parameter change

#

This might be a PIXI issue since Filter is annotated as extending Shader

#

So they're just doing naughty things with JS

#

@rigid zinc if you've got an opportunity to look that would be appreciated

wide hollow
#

For Atro: I think the simplest option is expand BaseShaderMixin's type documentation to typeof PIXI.Shader | PIXI.Filter like the AdaptiveFragmentChannelMixin type annotation

wide hollow
#

Feature request: Could one of the next few v11 releases also ship with generated types? I'm not sure what process changes you're making and if that's possible, but it would be a huge help in going to v12 if one of the closing v11 releases had your generated types that we could diff with the v12 release

rigid zinc
#

I think you misunderstand

#

we can always generate the types ourselves

vast horizon
#

Afraid not, it’s too big of a process change

rigid zinc
#

Yeah if you meant the larger changes it'd be difficult to backport

#

but if you simply meant getting .d.ts files

wide hollow
rigid zinc
#

you can do that with tsc --emitDeclaration plus a few other flags I'm forgetting

rigid zinc
#

At least that's my expectation

wide hollow
#

moot anyways with it being too big of a process change

vast horizon
#

well, what LukeAbby is mentioning does work (today) in v11

#

so that isn't a process change

wide hollow
#

yeah it's all doable

verbal quartz
#

given the documentation improvements that we're going to see (I think) we should get closer and closer to what we've built by hand so far

rigid zinc
#

Yes and no

#

It'll definitely get closer over time so that much is true

#

But the no comes in by the fact that what we've built by hand is designed to interface around things that Foundry itself will probably never type

#

from somewhat more frivolous stuff like mergeObject all the way to DataModel.

#

I don't think DataModel will ever get to the point of being automatically documented inside of Foundry's documentation

#

unless they literally lift the mixin and interface declaration merging code from fvtt-types. Which frankly I'd be happy to help them support because DataModel properties NOT being documented is an annoying hole. But at that point, for the purposes of documentation, they would be bundling actual TypeScript files.

#

(as soon as I can get DataModel to stop being too nested or something... for some inexplicable reason)

verbal quartz
#

we also have stuff like the flag config, setting config etc. we're never gonna get there wholesale, of course

rigid zinc
#

Yeah I suppose one of the things I meant to point out is that in some areas refining Foundry's types might take it further from what we've done by hand

#

because they will take different compromises

wide hollow
#

Not sure where to put this, but while doing this I'm noticing some inconsistencies in linting and annotations in the source files — shaders/effects/lighting.js in particular is a bit all over the place, probably because of all the template strings?

vast horizon
wide hollow
#

This one was small, but there's a difference in spacing in the defaultUniforms object call

class FlameIlluminationShader extends AdaptiveIlluminationShader {
  /** @inheritdoc */
  static defaultUniforms = ({...super.defaultUniforms, brightnessPulse: 1});
}

vs.


class FlameColorationShader extends AdaptiveColorationShader {
  /** @inheritdoc */
  static defaultUniforms = ({ ...super.defaultUniforms, brightnessPulse: 1});
}
wide hollow
#

And then generally the whole thing is a bit haphazard with what gets inheritdoc commented for all the static fragmentShader overrides

rigid zinc
#

Well tbf, inheritDoc is doing essentially nothing for them arm so I can understand the haphazard application

#

But yeah for the future it should be documented

wide hollow
#

Cut it, don't cut it, I don't really care either way but I just wanted to highlight that file has some very inconsistent formatting

tall pebble
#

Just as a note too TypeDoc doesn't support @inheritdoc; it only recognizes @inheritDoc.

wide hollow
#

yeah we've had that discussion already separately

wide hollow
#

@stray iron if you're interested in further code cleanup; it seems like there's some arbitrary differences in how these two create methods are implemented, for example the program line calling another static method vs. doing it inline, as well as the use of foundry.utils.mergeObject vs. dot notation (the latter I might be misunderstanding a distinction in the implementation between the two)

class AbstractBaseShader {
  /**
   * A factory method for creating the shader using its defined default values
   * @param {object} initialUniforms
   * @returns {AbstractBaseShader}
   */
  static create(initialUniforms) {
    const program = PIXI.Program.from(this.vertexShader, this.fragmentShader);
    const uniforms = foundry.utils.mergeObject(this.defaultUniforms, initialUniforms,
      {inplace: false, insertKeys: false});
    const shader = new this(program, uniforms);
    shader._configure();
    return shader;
  }
}

class AbstractWeatherShader extends AbstractBaseShader {
  /** @override */
  static create(initialUniforms) {
    const program = this.createProgram();
    const uniforms = {...this.commonUniforms, ...this.defaultUniforms, ...initialUniforms};
    return new this(program, uniforms);
  }

  /* -------------------------------------------- */

  /**
   * Create the shader program.
   * @returns {PIXI.Program}
   */
  static createProgram() {
    return PIXI.Program.from(this.vertexShader, this.fragmentShader);
  }
}
wide hollow
#

Actual types issue - transforming fragmentShader into a function just for FogShader is... odd? I'm sure there were ticket notes for why this is the case, but it's going to crop up as an issue with any typings

class AbstractBaseShader {
  static fragmentShader: string;
}

class AbstractWeatherShader extends AbstractBaseShader {}

class FogShader extends AbstractWeatherShader {
  static fragmentShader(mode: number): string;
}
wide hollow
wide hollow
#

Another type issue, present in v11.315 and v12p1; PlaceableObject.clear() returns this, but Tile.clear() returns void

vast horizon
#

@rigid zinc I've got a typedoc question for you which is not urgent but could be helpful.

I would ideally use the following pattern:

/**
 * @typedef {Object} ActiveEffectData
 * ... omitted for brevity ...
 */

/**
 * The ActiveEffect document.
 * @mixes {@link ActiveEffectData}
 */
export default class BaseActiveEffect extends Document {
  /**
   * Construct an ActiveEffect document using provided data and context.
   * @param {Partial<ActiveEffectData>} data        Initial data from which to construct the ActiveEffect
   * @param {DocumentConstructionContext} context   Construction context options
   */
  constructor(data, context) {
    super(data, context);
  }

This works within the IDE where the ActiveEffect document will recognize that it has the properties which are described in ActiveEffectData, but this doesn't work for doc generation since it's JSDoc only https://jsdoc.app/tags-mixes.

Is there any clever trick I can use to avoid having to write ActiveEffectData twice, once as a declared type and once as properties of the BaseActiveEffect class?

rigid zinc
#

Right so there's a few options and doing it exactly right is difficult. Thankfully I doubt you'll run into the issues I did when tackling this problem since that involved some very specific issues.

Here's what I wanted to be able to do for you:

interface BaseActiveEffect extends ActiveEffectData {}

It's a oneliner and it should work. This does something called Declaration Merging if you're unfamiliar. I don't think we can use it though because I can't seem to emit an interface "out of nothing" though. I can emit it if I'm playing with real types already.

This is what I'd do if I were writing this in TypeScript. Well that's not quite true I literally worked on doing this exact thing so what I'd really be doing is writing this to calculate ActiveEffectData based off of the schema to avoid having to maintain different versions of ActiveEffectData (one for input, one for source, one for data, etc.) but that's beside the point.

I'm mentioning it because I do have a solution, it's just very inelegant.

#

Basically what I did was create a fake mixin that does nothing at runtime and then I provided it, at the type level, the ActiveEffectData to merge in.

#

I honestly want to do this better but that basically reduces to seeing if you can do declaration merging in TSDoc

vast horizon
#

Thank you for taking a look! It’s cool that you came up with a solution

#

I think, as you say, the solution is a little unpleasant though

#

Probably this is the kind of thing that might best to do with a custom typedoc plugin, I haven’t spent the time yet to learn how to make one

rigid zinc
#

I'll see if I can do better but a lot of this has ended up me trying to backtrack my TS knowledge into TypeDoc knowledge

#

Hmm, yeah I could see a plugin doing this work

vast horizon
#

The @mixes {typedef} property is retained in the generated dts files, so teaching that tag to have some behavior of attaching the properties of the typedef to the documented class should (I think?) be possible?

rigid zinc
#

possibly? I was thinking a plugin that makes @mixes {typedef} turn into interface [ClassName] extends [typedef]

#

The reason why is teaching the tag to have the behaviour of attaching the properties could be quite complicated

#

or quite simple!

#

hard to know until I see the architecture but it might be that they defer to something like tsc to find out the properties on a symbol and whether they let plugins mess with that or not is unclear to me without diving into it

verbal quartz
rigid zinc
#

Yeah I was looking at the docs too yesterday and I figure it's possible in some capacity

#

I see it's possible to find all custom tags, so find @mixes and then, well mix it into the documented properties

vast horizon
#

@rigid zinc any advice on the best way to document this paradigm?

class ParentClass {
  /**
   * @param {ParentClassThing} someThing
   */
  constructor(someThing) {
    /**
     * @type {ParentClassThing}
     */
    this.thing = someThing;
  }
}

/**
 * The type of ChildClass#thing should be ChildClassThing. How to do?
 */
class ChildClass extends ParentClass {
  /**
   * @param {ChildClassThing} someThing
   */
  constructor(someThing) {
    super(someThing);
  }
}
#

I would like the documentation to understand that ChildClass#thing is has the type ChildClassThing not ParentClassThing

rigid zinc
#

Ah! Lemme take a look

vast horizon
#

Ideally I wouldn't even have to re-document the constructor, but that's bonus points

#

I assume it will somehow involve @template, but I don't know how to do it

rigid zinc
#

Yeah, I think that'll be the trick. Fortunately it's all instance side so it's quite doable.

#

I know how to write it in TS already

#

just need to reverse engineer the JSDoc

#
/**
 * @template {ParentClassThing} SomeThing
 */
class ParentClass {
  /**
   * @param {SomeThing} someThing
   */
  constructor(someThing) {
    /**
     * @type {SomeThing}
     */
    this.thing = someThing;
  }
}

/**
 * The type of ChildClass#thing should be ChildClassThing. How to do?
 * @extends {ParentClass<ChildClassThing>}
 */
class ChildClass extends ParentClass {}
#

If you leave off the constructor entirely in ChildClass you won't have to redocument it

#

if you actually have a real constructor in the child class you'll probably have to redocument it because @inheritdoc and the like probably won't work.

#

I believe this'd do it

vast horizon
#

I'll try

#

thank you!

#

is it possible for the parent class to have more than one templated attribute?

rigid zinc
#

Yes

vast horizon
#

How would the @extends block be written in the case where the parent class has two templates, one for someThing and another for otherThing?

#

Hmm... the IDE (WebStorm) is giving me a bit of grief about trying to use @template in this way:

#

oh, wait, maybe I did it backwards?

#

aah yeah oops nm

rigid zinc
# vast horizon How would the `@extends` block be written in the case where the parent class has...

Hmmm... I feel like there could be a more idiomatic TSDoc way but this'd be how

/**
 * @template {ParentClassThing} SomeThing1
 * @template {ParentClassOtherThing} OtherThing
 */
class ParentClass {
  /**
   * @param {SomeThing} someThing
   * @param {OtherThing} otherThing
   */
  constructor(someThing, otherThing) { ... }
}

/**
 * The type of ChildClass#thing should be ChildClassThing. How to do?
 * @extends {ParentClass<ChildClassThing, ChildClassOtherThing>}
 */
class ChildClass extends ParentClass {}
vast horizon
#

gotcha, so the templated values are just declared comma-separated in the order that @templates are defined?

rigid zinc
#

Yeah, under the hood you get this TS

#
/**
 * @template {ParentClassThing} SomeThing1
 * @template {ParentClassOtherThing} OtherThing
 */
declare class ParentClass<SomeThing1 extends ParentClassThing, OtherThing extends ParentClassOtherThing> {
    /**
     * @param {SomeThing} someThing
     * @param {OtherThing} otherThing
     */
    constructor(someThing: SomeThing, otherThing: OtherThing);
}
/**
 * The type of ChildClass#thing should be ChildClassThing. How to do?
 * @extends {ParentClass<ChildClassThing, ChildClassOtherThing>}
 */
declare class ChildClass extends ParentClass<ChildClassThing, ChildClassOtherThing> {
    /**
     * @param {SomeThing} someThing
     * @param {OtherThing} otherThing
     */
    constructor(someThing: SomeThing, otherThing: ChildClassOtherThing);
}
#

As you can see it sets up two generic parameters on the parent class in the order the @templates are defined

#

working entirely within TS you'd get intellisense for the shape of extends ParentClass

#

like you'd be shown there's two generic parameters immediately

#

But since a lot of the details are hidden the line @extends {ParentClass<ChildClassThing, ChildClassOtherThing>} in the TSDoc is admittedly not the most obvious thing to reach for

frail anvil
#

Not sure where else to bring this up but since it's documentation related thought this place may fit.
Maybe this is intended, and most certainly it is nitpicky, but it confused me, so if it ain't intended may as well flag it.

Both TypeDataField and DocumentTypeField take in a class as a first param, and both have the exact same JSDoc explanation of "The base document class which belongs in this field"

However on DocumentTypeField the param is called documentClass and on TypeDataField it's called document. Due to the inconsistent naming I had a brief "wait are these different?" but as is evident from the implementation of f.e. BaseActiveEffect.defineSchema they seem to be the same as both take input this

wide hollow
#

Q: Are these emitted types supposed to be in the foundry files that ship? or is this goal more of a "We want auto-generated types to be higher quality"

verbal quartz
#

I think it was the latter, purely for the documentation generation. There's no plans to ship types IIRC

vast horizon
wide hollow
#

Has anyone else had success trying typescript's jsdoc-to-.d.ts built in conversion for V12? The issue I ran into is that running on public creates files that are too big. The errors if I run on client/client-esm/common aren't super helpful

wide hollow
#

Digging into the native emitter; it's not handling the mixins at all, which is obviously Not Great

vast horizon
#

documentation for mixins is a major pain point that we don't really have a solution for

wide hollow
#

yeah it seems like the problem with basically every automatic documentation effort is as soon as something passes through a mixin - which all the really important stuff does - you break your inheritance

#

in a fully implemented .d.ts file it's not too bad, but I don't think JSDoc works with mixins well at all

azure rain
verbal quartz
#

At this point JSDoc and TSDoc should probably be considered as two entirely different things.

rigid zinc
#

Agreed. I got mixins largely working for Atropos but completely working is difficult given the upstream fixes required

fresh kernel
#

Can someone tell me if there is already a types library I can devDependencies yet? The whole point for JSDoc/TSDoc was to generate it and package it, but cant find any meaninful guide for this

wide hollow
#

We got over the big hump to make data models work. I'm expecting v12 to be much quicker and easier.

#

Through a combination of sweat, blood, tears, and upstream fixes in the typescript compiler, I believe we finally have a working implementation of properly types DataFields

fresh kernel
#

I was hoping for an official implementation since the JSDoc is there. You guys have been doing an amazing (and quite frankly much needed) work in lowering the try and error of the API

#

I see how automation tasks could be de-prioritized, but once in place this should be a problem of the past, once automated API version gets generated

wide hollow
#

Mixins are the area that's most obviously broken

fresh kernel
#

I totally see that happening, maybe devs and shine a light on this

echo stump
vast horizon
#

To be absolutely clear: we are not using JSDoc for documentation generation.

#

JSDoc is - unfortunately - an abandoned project which doesn't support modern ESM features

#

Typescript - in theory - supports modern ESM features because, as everyone loves to tell me, JS is valid TS. Except the times that it can't, or doesn't, or won't.

#

Typedoc is flawed, but it's better than JSDoc in terms of the quality of documentation it generates for us

#

It's still a pain in the ass though.

fresh kernel
#

Will it help to generate the types needed (as per the topic of this thread) or just HTML Documentation?

#

(serious question)

vast horizon
#

Also, to acknowledge in fairness, a lot of the times that the typedoc that gets generated is not good is because we are not using it properly.

wide hollow
#

Yeah, getting the TS typings correct has required a handful of compromises. One big issue is we don't have working pass throughs for generics on the mixins.

wide hollow
vast horizon
#

it doesn't choke. We can run npx tsc on our full client-side code without issue

wide hollow
#

I did see you guys closed out the ticket for the bug with the client code

vast horizon
#

the generate types are not as useful as most devs want though.

wide hollow
vague oyster
vast horizon
wide hollow
#

Grateful for this coming, lots of complaints the wiki links are broken

#

Anyways @fresh kernel I would be immensely grateful to anyone who helps me with the Types project

fresh kernel
#

I do want to be able to spin up a parcel lib project and just add the types and be ready for making my module, sending many of thos to any or partialy mock the APi to be consistent y hurting my will to maintain and make improvements to my mod TBH

fresh kernel
fresh kernel
#

Nope

wide hollow
rigid zinc
vast horizon
#

Yeah I saw that. I am very interested in that

#

It’s a much nicer way to import typedefs

#

We’ll plan to incorporate this new convention in v13 as we move all the rest of our code to ESM

rigid zinc
#

Sweet!

#

Have there been any new problems with TSDoc btw?

vast horizon
#

Always