kit icon indicating copy to clipboard operation
kit copied to clipboard

[email protected] ambient.d.ts breaks DOM manipulation function types

Open tv42 opened this issue 3 years ago • 3 comments

Describe the bug

(This bug also occurs with @sveltejs/[email protected], I'm talking about 1.0.0-next.36 because that's what introduced the issue.)

I need a workaround for an ugly Safari bug, which forces me to do some HTML rewriting before I use @html. I use DOMParser.parseFromString and some light DOM manipulation for this. This function is only called when browser from $app/environment is true.

Upgrading to @sveltejs/[email protected] broke the Typescript DOM API:

Error: Argument of type 'HTMLDivElement' is not assignable to parameter of type 'Content'. (ts)
      div.textContent = "\n";
      child.after(div);
    }

That child there is of type Element, and Element.after absolutely should take a HTMLDivElement. Looking up the type definition of child.after in an IDE leads to node_modules/@cloudflare/workers-types/index.d.ts which has the incorrect

declare type Content = string | ReadableStream | Response;
...
interface Element {
   ...
  after(content: Content, options?: ContentOptions): Element;

It seems PR https://github.com/sveltejs/kit/pull/6917 caused ambient.d.ts referencing Cloudflare's types make them override the DOM API types: https://github.com/sveltejs/kit/blob/4b7fc72fc57db3c893b1240611611c36eb1f95a6/packages/adapter-cloudflare-workers/ambient.d.ts#L1

It seems this only triggers if the tsconfig workspace contains a file that actually imports @sveltejs/adapter-cloudflare. In my case, I have svelte.config.js added to tsconfig so I get useful IDE feedback.

Reproduction

https://github.com/tv42/sveltekit-issue-cloudflare-dom

The latest commit has svelte-check complaining

/blah/bug-dom-worker-2/src/routes/+page.svelte:9:17
Error: Argument of type 'HTMLSpanElement' is not assignable to parameter of type 'Content'. (ts)
      span.innerText = "dynamic content";
      h1.append(span);
	}

You can checkout the commit before it switched to @sveltejs/[email protected], run npm i, and observe npm run check be happy.

Note how triggering this hinges on the commit that edited tsconfig.json.

Logs

No response

System Info

System:
    OS: Linux 6.0 NixOS 22.11 (Raccoon) 22.11 (Raccoon)
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
    Memory: 36.27 GB / 62.60 GB
    Container: Yes
    Shell: 5.1.16 - /run/current-system/sw/bin/bash
  Binaries:
    Node: 16.18.1 - /nix/store/gqbqpgmr7s8b3km14m54ys5kilx0akcc-nodejs-16.18.1/bin/node
    npm: 8.19.2 - /nix/store/gqbqpgmr7s8b3km14m54ys5kilx0akcc-nodejs-16.18.1/bin/npm
  Browsers:
    Firefox: 108.0.1
  npmPackages:
    @sveltejs/adapter-cloudflare: ^1.0.0-next.36 => 1.0.0-next.36
    @sveltejs/kit: ^1.0.0 => 1.0.1
    svelte: ^3.54.0 => 3.55.0
    vite: ^4.0.0 => 4.0.3

Severity

blocking an upgrade

Additional Information

No response

tv42 avatar Dec 27 '22 19:12 tv42

Presumably you can use a @ts-ignore or @ts-expect-error comment as a workaround and avoiding this blocking an upgrade?

Conduitry avatar Dec 27 '22 19:12 Conduitry

Yes, if I litter enough @ts-expect-error in the source it stops complaining. I can make progress now, thank you.

(It still shouldn't complain, and now it's just a minefield of what web standard API does Cloudflare break next.)

tv42 avatar Dec 27 '22 19:12 tv42

This also broke a place where I do

const result = (await response.json()) as MyType;

with eslint complaining

  58:18  error  This assertion is unnecessary since it does not change the type of the expression  @typescript-eslint/no-unnecessary-type-assertion

and that seems to be because Cloudflare overrides response.json to be

  json<T>(): Promise<T>;

while node_modules/typescript/lib/lib.dom.d.ts says

interface Body {
    readonly body: ReadableStream<Uint8Array> | null;
    readonly bodyUsed: boolean;
    arrayBuffer(): Promise<ArrayBuffer>;
    blob(): Promise<Blob>;
    formData(): Promise<FormData>;
    json(): Promise<any>;
    text(): Promise<string>;
}

This spooky action at a distance property is like the worst part of Typescript :-(

tv42 avatar Dec 27 '22 20:12 tv42