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

Auto-Generated Types #112

Merged
merged 11 commits into from
Sep 30, 2021
Merged

Auto-Generated Types #112

merged 11 commits into from
Sep 30, 2021

Conversation

mrbbot
Copy link
Contributor

@mrbbot mrbbot commented Sep 24, 2021

Types are now automatically generated with releases by parsing the runtime's source code, and merging in overrides/docs defined in this repository (for generics, overloads, comments, etc).

webworker no longer needs to (and shouldn't) be included in tsconfig lib.

The final merged AST that's used to render the TypeScript types, workers.json, is also included in the repository. This could be used to generate bindings for other languages that compile to WebAssembly. Rust output is coming soon.

Closes: #55, #75, #76, #81, #84, #96, #97, #100, #101, #102, #105, #106, #107, #108

@koeninger
Copy link
Contributor

Exciting stuff, couple of questions

  • What's the easiest way for users to test this out in their projects?
  • Can you say a little more about changing interface to abstract class in some cases? That's technically a breaking change if someone was implementing an interface for mocking or whatever, right?

@mrbbot
Copy link
Contributor Author

mrbbot commented Sep 24, 2021

What's the easiest way for users to test this out in their projects?

Until this gets published as the npm package, user's could install it from Git:

$ npm install --save-dev cloudflare/workers-types#master
{
  "devDependencies": {
    "@cloudflare/workers-types": "cloudflare/workers-types#master"
  }
}

Then remove webworker from lib in tsconfig.json.


Can you say a little more about changing interface to abstract class in some cases? That's technically a breaking change if someone was implementing an interface for mocking or whatever, right?

This is to make the differentiation between classes that exist at runtime and structs that are just there for typing. Whilst this is definitely a breaking change, it's far from the only one (some things have been renamed, and some things have been removed that shouldn't have been there in the first place). This would have to be a major version bump anyways so I think it's ok.

@lukeed
Copy link
Contributor

lukeed commented Sep 24, 2021

Very cool 🎉

Yeah it looks to me like the abstract marker here is just a signifier that an existing class shouldn't be used directly. However, using Durable Object types as an example, this allows someone to try to do the following:

class MyStorage extends DurableObjectStorage {
  //
}

This would be allowed/okay according to the types, but unlike true abstract classes (like Body) the Durable Object classes don't exist in runtime.


This definitely warrants a new major version release, as it also requires tsconfig changes. @koeninger because of the major version bump, existing users will only get the new types of they manually update. This is common/expected throughout the ecosystem wrt types

@lukeed
Copy link
Contributor

lukeed commented Sep 24, 2021

@mrbbot Couple things / questions:

  1. Can we move the two .json files into something like a src directory to help indicate what they are?
  2. I know it wasn't there, but can you add a files list to the package.json to narrow which files get uploaded to the registry?
  3. Can you include some README instructions/overview so that maintainers know how this is assembled & so that all contributors know how/where to make any adjustments?

@mrbbot
Copy link
Contributor Author

mrbbot commented Sep 25, 2021

  1. Can we move the two .json files into something like a src directory to help indicate what they are?

➡️ Sure!

  1. I know it wasn't there, but can you add a files list to the package.json to narrow which files get uploaded to the registry?

🔎 Yep, I've included index.d.ts and the new src directory (so people can get the JSON from npm installing if they want to generate types for other languages).

  1. Can you include some README instructions/overview so that maintainers know how this is assembled & so that all contributors know how/where to make any adjustments?

📚 Done!

@evanderkoogh
Copy link
Contributor

Ohh.... this is so cool.

I changed my @cloudflare/workers-types version to cloudflare/workers-types#auto-generated, removed webworker from lib, ran yarn install and everything worked..

Merge.. Merge.. Merge!

@Electroid
Copy link

Do we want to use dates for releases, so it's easier for tools to grab types for a compatibility date? (e.g. 2021.08.01)

package.json Outdated Show resolved Hide resolved
Co-authored-by: Luke Edwards <luke.edwards05@gmail.com>
Copy link
Contributor

@lukeed lukeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

@Electroid I like the versioning idea & we should do that to accommodate the internal release process. I have notes from @mrbbot to hand off to @vlovich

We can do the version changes separately from this PR

@rabaut
Copy link

rabaut commented Sep 28, 2021

Using the new version is causing an error when installing with yarn:

invalid package version "2021.09.24"

Maybe this doesn't conform to the semantic versioning format allowed in packages?

@cannona
Copy link

cannona commented Sep 28, 2021

Maybe this doesn't conform to the semantic versioning format allowed in packages?

I've run into this on other projects. It has an issue with leading 0's.

@1000hz
Copy link
Contributor

1000hz commented Sep 28, 2021

Do we want to use dates for releases, so it's easier for tools to grab types for a compatibility date? (e.g. 2021.08.01)

Linking releases to compatibility dates is a good idea, but I think we should do it via npm dist-tags rather than hijacking the version with a non-semver value.

@lukeed
Copy link
Contributor

lukeed commented Sep 28, 2021

We'd have to do it without leading zeros. Semver (at least on npm) sorts by the numerical value, not by traditional character value sorting. (x.12.x is greater than x.8.x)

My hesitation with dist tags is that it kinda means we have two versioning schemes. And then we have to manually decide if every new release is semver major/minor/patch...or ignore semver entirely.

@rabaut
Copy link

rabaut commented Sep 29, 2021

Can we revert 33cd391 and release this? I agree with @lukeed that the version changes can be separate from this. This PR is a huge improvement so it would be great to get it released 🚀

@Electroid
Copy link

Electroid commented Sep 29, 2021

Removing the leading 0's seems simple enough: 2021.9.29. I agree, let's get this merged 🚀

@mrbbot
Copy link
Contributor Author

mrbbot commented Sep 29, 2021

Cool, I've removed the leading 0 and fixed the DurableObjectTransaction override. 👍

@threepointone
Copy link
Contributor

Can I merge this? Would be a nice thing to announce this week alongside birthday week (and I want to use the types so badly haha).

@mrbbot
Copy link
Contributor Author

mrbbot commented Sep 30, 2021

Go for it @threepointone! 😃 Just updated the changelog with the correct version.

@threepointone threepointone merged commit 61c5989 into master Sep 30, 2021
@threepointone
Copy link
Contributor

Merged! I'll do a major release later today/tomorrow, once I figure out how...

@rabaut
Copy link

rabaut commented Oct 9, 2021

@threepointone Any idea when the release will happen?

@threepointone
Copy link
Contributor

Hopefully this coming week.

declare abstract class DurableObjectState {
waitUntil(promise: Promise<void>): void;
readonly id: DurableObjectId | string;
readonly storage?: DurableObjectStorage;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is storage potentially undefined here? 🤔
What should workers do if storage is undefined?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know this is just leaking a bit of implementation detail, you should be able to assume that storage is defined. I'll add a pr to override that type.

@threepointone threepointone deleted the auto-generated branch November 2, 2021 10:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.