Typescript type errors

[jeremy-rifkin]

Hi there,
I'm running into loads of type errors from linkedom when using the library in a typescript project.

These type errors appear to all be either missing properties or incorrect implementations of base class methods (due to parameter types or return types not matching).

The errors are too long to send but I will send a few:

node_modules/linkedom/types/dom/parser.d.ts:12:5 - error TS2416: Property 'parseFromString' in type 'DOMParser' is not assignable to the same property in base type 'DOMParser'.
  Type '<MIME extends "image/svg+xml" | "text/html" | "text/xml">(markupLanguage: string, mimeType: MIME) => { "text/html": HTMLDocument; "image/svg+xml": SVGDocument; "text/xml": XMLDocument; }[MIME]' is not assignable to type '(string: string, type: DOMParserSupportedType) => Document'.
    Type 'HTMLDocument | SVGDocument | XMLDocument' is not assignable to type 'Document'.
      Type 'HTMLDocument' is missing the following properties from type 'Document': URL, alinkColor, anchors, applets, and 181 more.

12     parseFromString<MIME extends keyof {
       ~~~~~~~~~~~~~~~

node_modules/linkedom/types/html/anchor-element.d.ts:4:14 - error TS2420: Class 'import("C:/Users/felis/home/projects/test-linkedom/node_modules/linkedom/types/html/anchor-element").HTMLAnchorElement' incorrectly implements interface 'HTMLAnchorElement'.
  Type 'HTMLAnchorElement' is missing the following properties from type 'HTMLAnchorElement': charset, coords, hreflang, name, and 132 more.

4 export class HTMLAnchorElement extends HTMLElement implements globalThis.HTMLAnchorElement {
               ~~~~~~~~~~~~~~~~~

...

node_modules/linkedom/types/html/document.d.ts:6:9 - error TS2416: Property 'all' in type 'HTMLDocument' is not assignable to the same property in base type 'HTMLDocument'.
  Property 'namedItem' is missing in type 'NodeList' but required in type 'HTMLAllCollection'.

6     get all(): NodeList;
          ~~~

  ../../../AppData/Roaming/npm/node_modules/typescript/lib/lib.dom.d.ts:5953:5
    5953     namedItem(name: string): HTMLCollection | Element | null;
             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    'namedItem' is declared here.

node_modules/linkedom/types/html/document.d.ts:15:9 - error TS2416: Property 'title' in type 'HTMLDocument' is not assignable to the same property in base type 'HTMLDocument'.
  Type 'HTMLTitleElement' is not assignable to type 'string'.

15     set title(arg: HTMLTitleElement);
           ~~~~~

...

Workaround: tsc --skipLibCheck

It'd be great to get the types correct here so --skipLibCheck isn't needed. I would make a PR for this however it seems design decisions would need to be made to fix the types. I'm not sure if the types can be reconciled while still extending / implementing globalThis.DOMParser, globalThis.HTMLElement, /globalThis\.HTML.*Element/, etc.

[WebReflection]

Is this related to latest TS release? AFAIK many use this project with TS and few PRs landed already and they were happy.

About design decisions if we’re talking TSDoc better hints and types I’d be more than happy to improve but if it means code needs to change to reflect 1:1 DOM API that might be out of this project scope and we should find a better way to tell TS to not bother there. Hope this makes sense.

[jeremy-rifkin]

Thanks for the reply, yeah this is the latest ts version.

The current types are great as far as working with the library in TS goes, the .d.ts files do provide a good summary of the library's interfaces. The issue is the .d.ts files claim to implement interfaces that aren't actually implemented (so it errors unless you use --skipLibCheck).

The design decision I wasn't sure about was just why the .d.ts files suggest these interfaces are implemented at all, I was puzzled by it as it seems outside the scope of the project. My only guess is that it was an attempt to let this library be used as a drop-in replacement for something like jsdom - but this seems dangerous.

Thank you for confirming that the goal for the library is not to implement a 1:1 DOM API. I will go ahead with making a PR.

[WebReflection]

@jeremy-rifkin are you going to update the PR?

[regexj]

Is there any update on this? I'm trying to test linkedom as a replacement for JSDOM in an existing typescript project but I'm getting lots of type errors on build. Hitting a wall.

[WebReflection]

I don't use TS and can't care less about it, simply because it produces JS so it's TS that should work, not JS that already does ... that being said, because I don't use TS, any PR is welcome here to fix whatever TS problem you have 👋

[WebReflection]

@regexj there's not much to thumb down in there ... this project is 100% tested, code covered, and downloaded 100K+ times per month, fueling Cloudflare Workers and much more ... do you realize the issue is not the code here, but your configuration? All TS hints have been mostly PRs because this modules is a JS one and it uses JSDoc with TS hints to help people like you ... moaning about TS issues without filing a PR is not much thumb up to me, sorry.

[regexj]

The thumbs down is to your attitude. I don't have time to get your library working in my TS environment. It's a shame, I wanted to give it a go as it sounded promising, but there are other JSDOM replacements to try. Good luck and hopefully you can get a contributor to help with your TS issues in the future.

[WebReflection]

I don't have time to get your library working in my TS environment

too bad .. as Open Source should be more pro-activeness and less demanding.

It's a shame

what is a shame is people demanding in Open Source, needing that software to move forward, and complaining without contributing around projects that don't fix automatically stuff for you.

I have other things to think about, but I am welcoming PRs related not to bugs, but to your environment that causes issues ... no PRs? sorry, I have other priorities piled up already.

[jeremy-rifkin]

Hey all, I'm sorry I've let my PR go stale. I'd like to get it merged so at least most errors while using it with TS can be resolved! I'll look at it again this weekend.

[jeremy-rifkin]

The tricky thing I ran into in #168 was the whole masquerading as globalThis.DOMParser or whatnot. The most correct thing to do would probably be to go add jsdoc type annotations throughout the whole library but that would be a lot of work.

Out of curiosity @WebReflection, would you be open to converting some of the library's code to typescript? Over on https://github.com/compiler-explorer/compiler-explorer we've had a long-haul effort to convert the codebase to typescript (almost there!) and in the process we've found a lot of bugs we never knew we had. I know it's a lot to ask so no worries at all if you'd like to avoid such a change :)

[WebReflection]

would you be open to converting some of the library's code to typescript?

no, for the same reason Deno core is JS and not TS, sorry.

[WebReflection]

we've found a lot of bugs we never knew we had

bug from here or in your logic? this one I am curious about, as 100% code coverage might not test all possible cases anyway, so I am willing to improve that, if bugs are here, and not just in TS.

lately many complained about bugs that don't exist in my source code, only in the TS env 🤷

[jeremy-rifkin]

bug from here or in your logic? this one I am curious about, as 100% code coverage might not test all possible cases anyway, so I am willing to improve that, if bugs are here, and not just in TS.

Separate from this project. We've found it's just extremely helpful to have a solid type foundation throughout the code, which is what typescript gives us. It has helped us identify issues where different parts of the code call functions with unexpected types (e.g. calling a function taking a string with a number or null or undefined) and issues where different parts of the code think they're working with slightly different types. Granted, CE is a much larger project that does not have 100% test coverage.

[WebReflection]

Here CE is 100% test covered … the DOM does a lot of coercion though … numbers or booleans are casted as string and accepted. This project won’t change the dynamic nature of the platform it represents, that’s on TS to be relaxed about it or ignore it. Were all these bugs a TS only issue though? I assume yes, but if not, please file those bugs here.