Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using official Cloudflare types #156

Closed
eugene1g opened this issue Apr 9, 2022 · 4 comments
Closed

Using official Cloudflare types #156

eugene1g opened this issue Apr 9, 2022 · 4 comments

Comments

@eugene1g
Copy link

eugene1g commented Apr 9, 2022

Currently, worktop hardcodes TypeScript types for request.cf at https://github.com/lukeed/worktop/blob/next/packages/worktop/src/cfw.d.ts. Over time, these static types drift from the actual implementation. For example, we're missing fields like clientTcpRtt and asOrganization. Cloudflare now publishes @cloudflare/workers-types, which has automatically generated TS types based on their current APIs, and they tend to be relatively up-to-date. Perhaps worktop could add @cloudflare/workers-types as a peer dependency, remove hardcoded types from src/cfw.d.ts, and use the link /// <reference types="@cloudflare/workers-types" /> when you need to refer to the official CF types?

@lukeed
Copy link
Owner

lukeed commented Apr 12, 2022

worktop is meant to be completely self-sufficient. You can/should use it directly and have information be accurate and up to date. In fact, until @cloudflare/workers-types 2.0 release, the worktop types for CF Workers were more accurate than the 1.x types.

worktop can't and won't use the CF types package because:

  1. worktop isn't exclusively a CF framework
  2. the CF types are globals and are meant only for a Cloudflare Workers environment
    This means that the type information included would be inaccurate for Deno, for example, and global types are all-or-nothing.
  3. the CF types are not 1:1 with the shared global names
    The types reflect the CF runtime, which is great, but if there are any spec compliance issues, they'll collide with TS internals and report as errors.
  4. Because of (3) you can't actually use @cloudflare/workers-types alongside the "dom" or "webworker" built-in types.

@lukeed lukeed closed this as completed Apr 12, 2022
@eugene1g
Copy link
Author

eugene1g commented Apr 12, 2022

I understand that one goal for worktop is to be runtime-agnostic. However, the project currently defines types for request.cf this way:

interface Request {
cf: IncomingCloudflareProperties;
}

This cf field is specific to Cloudflare, and if developers access req.cf they expect this property to reflect the behavior of Cloudflare Workers (what fields exist, what is nullable/optional, etc). The issue is that IncomingCloudflareProperties is hardcoded, and has become outdated. It seems that worktop cannot choose not to ship this type for Cloudflare Workers, it can only choose between "duplicate, and maintain regularly" or "reference the official definitions maintained by Cloudflare".

I found two ways around this issue:

  1. Cast the cf property manually in every handler, e.g. const cf = req.cf as unknown as IncomingRequestCfProperties;
  2. Override the type within worktop, e.g. in global.d.ts:
/// <reference types="@cloudflare/workers-types" />

declare module "worktop/cfw" {
  interface IncomingCloudflareProperties extends IncomingRequestCfProperties {}
}

I don't think either solution is what you want developers to do, and since Cloudflare recommends new projects to use @cloudflare/workers-types, these conflicts will become more common. Embracing @cloudflare/workers-types should reduce your long-term burden for worktop as Cloudflare now actively maintains that package.

@lukeed
Copy link
Owner

lukeed commented Apr 12, 2022

What request.cf property is outdated? Let’s just fix that. It changes infrequently.

@eugene1g
Copy link
Author

Cool, I'll do a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants