Skip to content
This repository has been archived by the owner on Feb 7, 2023. It is now read-only.
This repository has been archived by the owner on Feb 7, 2023. It is now read-only.

Using with a Workers SSR React app #164

Closed
kentcdodds opened this issue Dec 1, 2021 · 12 comments
Closed

Using with a Workers SSR React app #164

kentcdodds opened this issue Dec 1, 2021 · 12 comments
Labels
v4 For the next major version

Comments

@kentcdodds
Copy link

kentcdodds commented Dec 1, 2021

Hey Cloudflare friends 👋

So I found a bit of a TypeScript problem with SSR React and Cloudflare. Based on workers-types, you shouldn't include the built-in DOM types on a Workers project which makes sense because workers isn't a DOM environment. The problem is that some of the code we're writing runs in the worker and some of it runs in the browser.

So I do need both types available in different parts of my file. Unfortunately AFAIK is not possible with TypeScript. So we need to have both types available for the whole file. Unfortunately, when I include DOM types with @cloudflare/workers-types, I get a bunch of errors because the types aren't compatible.

I'm not sure what to do about this. Have y'all faced this before?

Here's what I get when I try to have both DOM and `@cloudflare/workers-types` at the same time:
node_modules/typescript/lib/lib.dom.d.ts:25:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: AbortController, AbortSignal, Blob, BodyInit, Cache, CacheStorage, CloseEvent, Crypto, CryptoKey, DOMException, Event, EventListener, EventListenerOrEventListenerObject, EventTarget, File, FormData, Headers, HeadersInit, MessageEvent, PromiseRejectionEvent, ReadableStream, ReadableStreamDefaultReader, Request, Response, StreamPipeOptions, SubtleCrypto, TextDecoder, TextEncoder, TransformStream, URL, URLSearchParams, WebSocket, WebSocketEventMap, WritableStream, WritableStreamDefaultWriter, caches, console, crypto, self

25 interface AddEventListenerOptions extends EventListenerOptions {
   ~~~~~~~~~

  node_modules/@cloudflare/workers-types/index.d.ts:4:1
    4 declare class AbortController {
      ~~~~~~~
    Conflicts are in this file.

node_modules/typescript/lib/lib.dom.d.ts:287:5 - error TS2687: All declarations of 'privateKey' must have identical modifiers.

287     privateKey?: CryptoKey;
        ~~~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:288:5 - error TS2687: All declarations of 'publicKey' must have identical modifiers.

288     publicKey?: CryptoKey;
        ~~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:650:5 - error TS2687: All declarations of 'kty' must have identical modifiers.

650     kty?: string;
        ~~~

node_modules/typescript/lib/lib.dom.d.ts:877:5 - error TS2687: All declarations of 'data' must have identical modifiers.

877     data?: T;
        ~~~~

node_modules/typescript/lib/lib.dom.d.ts:1701:5 - error TS2687: All declarations of 'read' must have identical modifiers.

1701     read?: number;
         ~~~~

node_modules/typescript/lib/lib.dom.d.ts:1702:5 - error TS2687: All declarations of 'written' must have identical modifiers.

1702     written?: number;
         ~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:3568:11 - error TS2430: Interface 'Comment' incorrectly extends interface 'CharacterData'.
  Types of property 'after' are incompatible.
    Type '(content: Content, options?: ContentOptions | undefined) => Comment' is not assignable to type '(...nodes: (string | Node)[]) => void'.
      Types of parameters 'content' and 'nodes' are incompatible.
        Type 'string | Node' is not assignable to type 'Content'.
          Type 'Node' is not assignable to type 'Content'.
            Type 'Node' is not assignable to type 'string'.

3568 interface Comment extends CharacterData {
               ~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:4632:101 - error TS2344: Type 'HTMLElementTagNameMap[K]' does not satisfy the constraint 'Element'.
  Type 'HTMLElement | HTMLAnchorElement | HTMLAreaElement | HTMLAudioElement | HTMLBaseElement | ... 63 more ... | HTMLFrameSetElement' is not assignable to type 'Element'.
    Type 'HTMLSelectElement' is not assignable to type 'Element'.
      Types of property 'remove' are incompatible.
        Type '{ (): void; (index: number): void; }' is not assignable to type '() => Element'.

4632     getElementsByTagName<K extends keyof HTMLElementTagNameMap>(qualifiedName: K): HTMLCollectionOf<HTMLElementTagNameMap[K]>;
                                                                                                         ~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:4872:11 - error TS2430: Interface 'Element' incorrectly extends interface 'ChildNode'.
  Types of property 'after' are incompatible.
    Type '(content: Content, options?: ContentOptions | undefined) => Element' is not assignable to type '(...nodes: (string | Node)[]) => void'.
      Types of parameters 'content' and 'nodes' are incompatible.
        Type 'string | Node' is not assignable to type 'Content'.

4872 interface Element extends Node, ARIAMixin, Animatable, ChildNode, InnerHTML, NonDocumentTypeChildNode, ParentNode, Slottable {
               ~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:4872:11 - error TS2430: Interface 'Element' incorrectly extends interface 'ParentNode'.
  Types of property 'append' are incompatible.
    Type '(content: Content, options?: ContentOptions | undefined) => Element' is not assignable to type '(...nodes: (string | Node)[]) => void'.
      Types of parameters 'content' and 'nodes' are incompatible.
        Type 'string | Node' is not assignable to type 'Content'.

4872 interface Element extends Node, ARIAMixin, Animatable, ChildNode, InnerHTML, NonDocumentTypeChildNode, ParentNode, Slottable {
               ~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:4922:14 - error TS2687: All declarations of 'tagName' must have identical modifiers.

4922     readonly tagName: string;
                  ~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:4953:101 - error TS2344: Type 'HTMLElementTagNameMap[K]' does not satisfy the constraint 'Element'.
  Type 'HTMLElement | HTMLAnchorElement | HTMLAreaElement | HTMLAudioElement | HTMLBaseElement | ... 63 more ... | HTMLFrameSetElement' is not assignable to type 'Element'.

4953     getElementsByTagName<K extends keyof HTMLElementTagNameMap>(qualifiedName: K): HTMLCollectionOf<HTMLElementTagNameMap[K]>;
                                                                                                         ~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:8111:11 - error TS2430: Interface 'HTMLSelectElement' incorrectly extends interface 'HTMLElement'.

8111 interface HTMLSelectElement extends HTMLElement {
               ~~~~~~~~~~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:14471:11 - error TS2430: Interface 'Text' incorrectly extends interface 'CharacterData'.
  Types of property 'after' are incompatible.
    Type '(content: Content, options?: ContentOptions | undefined) => Text' is not assignable to type '(...nodes: (string | Node)[]) => void'.
      Types of parameters 'content' and 'nodes' are incompatible.
        Type 'string | Node' is not assignable to type 'Content'.

14471 interface Text extends CharacterData, Slottable {
                ~~~~

node_modules/@types/react/index.d.ts:2869:78 - error TS2344: Type 'HTMLSelectElement' does not satisfy the constraint 'HTMLElement'.
  The types returned by 'remove()' are incompatible between these types.
    Type 'void' is not assignable to type 'Element'.

2869         select: DetailedHTMLFactory<SelectHTMLAttributes<HTMLSelectElement>, HTMLSelectElement>;
                                                                                  ~~~~~~~~~~~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:4:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: AbortController, AbortSignal, Blob, BodyInit, Cache, CacheStorage, CloseEvent, Crypto, CryptoKey, DOMException, Event, EventListener, EventListenerOrEventListenerObject, EventTarget, File, FormData, Headers, HeadersInit, MessageEvent, PromiseRejectionEvent, ReadableStream, ReadableStreamDefaultReader, Request, Response, StreamPipeOptions, SubtleCrypto, TextDecoder, TextEncoder, TransformStream, URL, URLSearchParams, WebSocket, WebSocketEventMap, WritableStream, WritableStreamDefaultWriter, caches, console, crypto, self

4 declare class AbortController {
  ~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:25:1
    25 interface AddEventListenerOptions extends EventListenerOptions {
       ~~~~~~~~~
    Conflicts are in this file.

node_modules/@cloudflare/workers-types/index.d.ts:98:12 - error TS2717: Subsequent property declarations must have the same type.  Property 'body' must be of type 'ReadableStream<Uint8Array> | null', but here has type 'ReadableStream<any> | null'.

98   readonly body: ReadableStream | null;
              ~~~~

  node_modules/typescript/lib/lib.dom.d.ts:2444:14
    2444     readonly body: ReadableStream<Uint8Array> | null;
                      ~~~~
    'body' was also declared here.

node_modules/@cloudflare/workers-types/index.d.ts:223:3 - error TS2687: All declarations of 'publicKey' must have identical modifiers.

223   publicKey: CryptoKey;
      ~~~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:223:3 - error TS2717: Subsequent property declarations must have the same type.  Property 'publicKey' must be of type 'CryptoKey | undefined', but here has type 'CryptoKey'.

223   publicKey: CryptoKey;
      ~~~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:288:5
    288     publicKey?: CryptoKey;
            ~~~~~~~~~
    'publicKey' was also declared here.

node_modules/@cloudflare/workers-types/index.d.ts:224:3 - error TS2687: All declarations of 'privateKey' must have identical modifiers.

224   privateKey: CryptoKey;
      ~~~~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:224:3 - error TS2717: Subsequent property declarations must have the same type.  Property 'privateKey' must be of type 'CryptoKey | undefined', but here has type 'CryptoKey'.

224   privateKey: CryptoKey;
      ~~~~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:287:5
    287     privateKey?: CryptoKey;
            ~~~~~~~~~~
    'privateKey' was also declared here.

node_modules/@cloudflare/workers-types/index.d.ts:376:3 - error TS2687: All declarations of 'tagName' must have identical modifiers.

376   tagName: string;
      ~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:377:12 - error TS2717: Subsequent property declarations must have the same type.  Property 'attributes' must be of type 'NamedNodeMap', but here has type 'IterableIterator<string[]>'.

377   readonly attributes: IterableIterator<string[]>;
               ~~~~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:4873:14
    4873     readonly attributes: NamedNodeMap;
                      ~~~~~~~~~~
    'attributes' was also declared here.

node_modules/@cloudflare/workers-types/index.d.ts:379:12 - error TS2717: Subsequent property declarations must have the same type.  Property 'namespaceURI' must be of type 'string | null', but here has type 'string'.

379   readonly namespaceURI: string;
               ~~~~~~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:4897:14
    4897     readonly namespaceURI: string | null;
                      ~~~~~~~~~~~~
    'namespaceURI' was also declared here.

node_modules/@cloudflare/workers-types/index.d.ts:430:84 - error TS2315: Type 'EventListener' is not generic.

430 declare type EventListenerOrEventListenerObject<EventType extends Event = Event> = EventListener<EventType> | EventListenerObject<EventType>;
                                                                                       ~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:434:70 - error TS2315: Type 'EventListenerOrEventListenerObject' is not generic.

434   addEventListener<Type extends keyof EventMap>(type: Type, handler: EventListenerOrEventListenerObject<EventMap[Type]>, options?: EventTargetAddEventListenerOptions | boolean): void;
                                                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:435:73 - error TS2315: Type 'EventListenerOrEventListenerObject' is not generic.

435   removeEventListener<Type extends keyof EventMap>(type: Type, handler: EventListenerOrEventListenerObject<EventMap[Type]>, options?: EventTargetEventListenerOptions | boolean): void;
                                                                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:641:3 - error TS2687: All declarations of 'kty' must have identical modifiers.

641   kty: string;
      ~~~

node_modules/@cloudflare/workers-types/index.d.ts:641:3 - error TS2717: Subsequent property declarations must have the same type.  Property 'kty' must be of type 'string | undefined', but here has type 'string'.

641   kty: string;
      ~~~

  node_modules/typescript/lib/lib.dom.d.ts:650:5
    650     kty?: string;
            ~~~
    'kty' was also declared here.

node_modules/@cloudflare/workers-types/index.d.ts:737:3 - error TS2687: All declarations of 'data' must have identical modifiers.

737   data: ArrayBuffer | string;
      ~~~~

node_modules/@cloudflare/workers-types/index.d.ts:737:3 - error TS2717: Subsequent property declarations must have the same type.  Property 'data' must be of type 'T | undefined', but here has type 'string | ArrayBuffer'.

737   data: ArrayBuffer | string;
      ~~~~

  node_modules/typescript/lib/lib.dom.d.ts:877:5
    877     data?: T;
            ~~~~
    'data' was also declared here.

node_modules/@cloudflare/workers-types/index.d.ts:838:3 - error TS2717: Subsequent property declarations must have the same type.  Property 'redirect' must be of type 'RequestRedirect | undefined', but here has type 'string | undefined'.

838   redirect?: string;
      ~~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:1513:5
    1513     redirect?: RequestRedirect;
             ~~~~~~~~
    'redirect' was also declared here.

node_modules/@cloudflare/workers-types/index.d.ts:1183:3 - error TS2687: All declarations of 'read' must have identical modifiers.

1183   read: number;
       ~~~~

node_modules/@cloudflare/workers-types/index.d.ts:1183:3 - error TS2717: Subsequent property declarations must have the same type.  Property 'read' must be of type 'number | undefined', but here has type 'number'.

1183   read: number;
       ~~~~

  node_modules/typescript/lib/lib.dom.d.ts:1701:5
    1701     read?: number;
             ~~~~
    'read' was also declared here.

node_modules/@cloudflare/workers-types/index.d.ts:1184:3 - error TS2687: All declarations of 'written' must have identical modifiers.

1184   written: number;
       ~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:1184:3 - error TS2717: Subsequent property declarations must have the same type.  Property 'written' must be of type 'number | undefined', but here has type 'number'.

1184   written: number;
       ~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:1702:5
    1702     written?: number;
             ~~~~~~~
    'written' was also declared here.

node_modules/@cloudflare/workers-types/index.d.ts:1238:42 - error TS2508: No base constructor has the specified number of type arguments.

1238 declare abstract class WebSocket extends EventTarget<WebSocketEventMap> {
                                              ~~~~~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:1248:50 - error TS2508: No base constructor has the specified number of type arguments.

1248 declare abstract class WorkerGlobalScope extends EventTarget<WorkerGlobalScopeEventMap> {
                                                      ~~~~~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:1278:102 - error TS2315: Type 'EventListenerOrEventListenerObject' is not generic.

1278 declare function addEventListener<Type extends keyof WorkerGlobalScopeEventMap>(type: Type, handler: EventListenerOrEventListenerObject<WorkerGlobalScopeEventMap[Type]>, options?: EventTargetAddEventListenerOptions | boolean): void;
                                                                                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@cloudflare/workers-types/index.d.ts:1300:105 - error TS2315: Type 'EventListenerOrEventListenerObject' is not generic.

1300 declare function removeEventListener<Type extends keyof WorkerGlobalScopeEventMap>(type: Type, handler: EventListenerOrEventListenerObject<WorkerGlobalScopeEventMap[Type]>, options?: EventTargetEventListenerOptions | boolean): void;
                                                                                                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 41 errors.
@geelen
Copy link
Contributor

geelen commented Dec 2, 2021

Interesting problem! Feels like Remix is introducing new semantics for a single TS file, a bit like the way Vue did with its single-file components that had pretty dreadful editor support to start with, but came good due to its popularity & ergonomics.

I wonder if you could end up with some kind of new TS syntax that supported this?:

import { useLoaderData } from "remix";
import type { LoaderFunction } from "remix";

//// <implicit types="@cloudflare/workers-types" />
export let loader: LoaderFunction = () => {
  return [{ name: "Pants" }, { name: "Jacket" }];
};

//// <implicit types="" lib="dom" />
export default function Products() {
  let products = useLoaderData();
  return (
    <div>
      <h1>Products</h1>
      {products.map(product => (
        <div>{product.name}</div>
      ))}
    </div>
  );
}

Or something... nicer than that? But feels to me like you're wanting the TS compiler to understand that a single file is actually two different flavours of TS side-by-side and they'll get split apart before they get run?

@kentcdodds
Copy link
Author

I think this has been a long-standing problem honestly. Ever since we started server rendering React we've had a file that could run in two different JS environments 🤷‍♂️

@geelen
Copy link
Contributor

geelen commented Dec 2, 2021

Yup, tbh I think this could even be an extension to this: microsoft/TypeScript#38511

Would be amazing to describe a module as requiring certain valid export types and that the body of each exported object be assumed to run in a particular environment. Might be a hard sell though...

The other idea is that your file gets typechecked twice, basically split (based on some pragma) into a .server.ts and .client.ts and then they're typechecked separately. That moves it from the realm of core TS changes into supporting-tooling changes, which might be more realistic to implement.

@mrbbot
Copy link
Contributor

mrbbot commented Dec 2, 2021

Hey! 👋 An extension to microsoft/TypeScript#38511 sounds awesome!

In the short term though, since I imagine this would require some pretty fundamental changes to TypeScript, it might be sensible to export a version of these types that are strictly additive (i.e. with just KV namespace, Durable Object, WebSocket accept() methods, etc), and behave more like @cloudflare/workers-types version 2 and below. This could still be auto-generated (check if a type is also defined in lib.webworker/lib.dom, and ignore conflicting definitions in @cloudflare/workers-types). This has the disadvantage of saying unimplemented APIs like MessageChannel are valid in worker code. If we were really concerned about this, we could always write an ESLint rule for detecting if a browser API was used in worker code, although...

This gets really complicated if a function is used on both the client and server:

import { useLoaderData } from "remix";
import type { LoaderFunction } from "remix";

export function helper() {
  // Is this typed with @cloudflare/workers-types or dom, or some intersection of both?
  // ...
  return 42;
}

//// <implicit types="@cloudflare/workers-types" />
export let loader: LoaderFunction = () => {
  return [{ name: "Pants", number: helper() }, { name: "Jacket" }];
};

//// <implicit types="" lib="dom" />
export default function Products() {
  let products = useLoaderData();
  return (
    <div>
      <h1>Products</h1>
      {products.map(product => (
        <div>{product.name}</div>
      ))}
      <p>Important number: {helper()}</p>
    </div>
  );
}

@kentcdodds
Copy link
Author

kentcdodds commented Dec 2, 2021

Yup, definitely looks like we're bumping up against the limits of TypeScript here 😅 I guess so far we've all just been fine with TypeScript not being able to catch when we're trying to access window in a server rendered component 🙃 The ideas presented here seem like good ideas to me.

@petebacondarwin
Copy link
Contributor

I just hit this problem, where I tried to use hightlight.js in a worker to generate some HTML that would be used in the browser. But that library explicitly references the DOM library types using /// <reference lib="dom" />, which causes the same conflicts discussed here.

If workers-types cannot be made compatible with the DOM types, which I "think" would solve this, perhaps we should make the workers-types non-global? In other words, we would import them explicitly using something like import {Response} from '@cloudflare/workers-types.

@jkup
Copy link

jkup commented Apr 14, 2022

Hey friends!

I wanted to recap what's going on here with some actionable steps people can take today as well as what we should do in the future to resolve this! Let me know if I miss anything or get something wrong.

The Problem: The issue here is that this package has global type definitions for a bunch of web APIs that conflict with the global type definitions for the same APIs provided by the dom and webworker type package. So if you add workers-types and dom or webworker you will get errors about conflicts.

What you can do today: The easiest thing you can do today is to remove the workers-types package from your Remix (or other) codebase. If you'd still like type safety around Cloudflare specific APIs like KV, you can copy them directly from https://github.com/cloudflare/workers-types/blob/master/index.d.ts or build your own simplified version.

What we need to do soon: In lieu of TypeScript changing it's support in this matter, we should either export a version of this package with only Cloudflare specific types (like @mrbbot mentioned) or change the package from global types to types that need to be imported explicitly (like @petebacondarwin mentions). Either way we'll need to do some work to get this package to play nice with projects that are already using dom and webworker.

Does that sound right to you all?

@shwao
Copy link

shwao commented Apr 30, 2022

I think

change the package from global types to types that need to be imported explicitly

sounds like a good idea!

@isaac-mcfadyen
Copy link

I'd just like to add on here for people looking at this in the future: I was actually getting errors because I didn't think to check if it was global or imported and just tried importing 🤦

Adding to tsconfig.json under compilerOptions.types worked.

@izznatsir
Copy link

Maybe we can make use of custom lib feature:
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-5.html

So we create an npm package that contains both lib dom and workers-type while removing dom types that are overrided by workers-types.

@mrbbot mrbbot added the v4 For the next major version label Oct 21, 2022
@mrbbot
Copy link
Contributor

mrbbot commented Oct 21, 2022

Hey! 👋 We're actively thinking about ways of fixing this in the next major version of workers-types using the new type generation system. 👍

@mrbbot
Copy link
Contributor

mrbbot commented Jan 19, 2023

Hey! 👋 As of @cloudflare/workers-types@4, you can selectively import type { ... } from "@cloudflare/workers-types". Importantly, this plays nice with lib.dom and lib.webworker. https://github.com/cloudflare/workerd will be the home for types going forward, so we're archiving this repository now. If there's future discussions that needs to be had, we can open a new issue over there. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
v4 For the next major version
Projects
None yet
Development

No branches or pull requests

8 participants