Add support for new bindings to wrangler types #5089

DaniFoldi · 2024-02-25T12:36:41Z

Fixes # [insert GH or internal issue number(s)].

What this PR solves / how to test:

This PR adds support for Email Send, Vectorize, Hyperdrive, mTLS, Browser Rendering and Workers AI bindings to wrangler types.

Author has addressed the following:

Tests
- Included
- Not necessary because:
Changeset (Changeset guidelines)
- Included
- Not necessary because:
Associated docs
- Issue(s)/PR(s):
- Not necessary because: adds support for bindings that should have already been supported i.e. addresses bug fix

Note for PR author:

We want to celebrate and highlight awesome PR review! If you think this PR received a particularly high-caliber review, please assign it the label highlight pr review so future reviewers can take inspiration and learn from it.

changeset-bot · 2024-02-25T12:36:44Z

🦋 Changeset detected

Latest commit: eed80d3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
wrangler	Patch
@cloudflare/vitest-pool-workers	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

dario-piotrowicz

LGTM 😃 🚀 ❤️

dario-piotrowicz · 2024-02-25T13:05:17Z

packages/wrangler/src/type-generation.ts

+	if (configToDTS.ai) {
+		// The @cloudflare/ai package types specify any, but from its usage
+		// we can infer it's a Fetcher type (await this.binding.fetch)
+		envTypeStructure.push(`${configToDTS.ai.binding}: Fetcher;`);


I'd be tempted to keep it as any if it is in @cloudflare/ai 🤔

I think this should probably be any/unknown since I'm not sure it's guaranteed to always be a Fetcher

I'd like to avoid generating any into user projects - lots of people use the recommended preset from typescript-eslint which includes no-explicit-any. unknown is somewhat better, I haven't thought of that myself. Let me push a fix

yes I would also go with unknown, I didn't suggest it as I don't know if @cloudflare/ai could complain about that?

Fortunately anything can be passed to any, including unknown: ts playground

Changed to unknown in b69e965

github-actions · 2024-02-26T11:58:53Z

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8158618486/npm-package-wrangler-5089

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/5089/npm-package-wrangler-5089

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8158618486/npm-package-wrangler-5089 dev path/to/script.js

Additional artifacts:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8158618486/npm-package-create-cloudflare-5089 --no-auto-update

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8158618486/npm-package-cloudflare-kv-asset-handler-5089

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8158618486/npm-package-miniflare-5089

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8158618486/npm-package-cloudflare-pages-shared-5089

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8158618486/npm-package-cloudflare-vitest-pool-workers-5089

Note that these links will no longer work once the GitHub Actions artifact expires.

wrangler@3.30.1 includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	workspace:*	3.20240223.0
`workerd`	1.20240223.1	1.20240223.1
`workerd --version`	1.20240223.1	2024-02-23

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

codecov · 2024-02-26T12:03:56Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.39%. Comparing base (04b37fc) to head (eed80d3).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5089      +/-   ##
==========================================
+ Coverage   70.31%   70.39%   +0.07%     
==========================================
  Files         298      298              
  Lines       15551    15567      +16     
  Branches     4001     4007       +6     
==========================================
+ Hits        10935    10958      +23     
+ Misses       4616     4609       -7

Files	Coverage Δ
packages/wrangler/src/type-generation.ts	`99.29% <100.00%> (+0.08%)`	⬆️

... and 6 files with indirect coverage changes

penalosa · 2024-02-26T16:15:35Z

packages/wrangler/src/type-generation.ts

+		send_email: config.send_email,
+		vectorize: config.vectorize,
+		hyperdrive: config.hyperdrive,
+		mtls_certificates: config.mtls_certificates,
+		browser: config.browser,
+		ai: config.ai,


Can we change the type of this object to make sure we catch issues like this in the future? It should be something like Omit<EnvironmentNonInheritable, "define" | "unsafe" | "tail_consumers" | "constellation" | "cloudchamber">

Added in 2e14c86

penalosa · 2024-02-26T16:15:39Z

packages/wrangler/src/type-generation.ts

+	if (configToDTS.ai) {
+		// The @cloudflare/ai package types specify any, but from its usage
+		// we can infer it's a Fetcher type (await this.binding.fetch)
+		envTypeStructure.push(`${configToDTS.ai.binding}: Fetcher;`);


I think this should probably be any/unknown since I'm not sure it's guaranteed to always be a Fetcher

mrbbot · 2024-03-04T16:13:44Z

@DaniFoldi looks like this is failing type checking? Would you be able to fix these failures?

DaniFoldi · 2024-03-04T17:57:45Z

Of course, I'll remove unsafe from the omit since the test actually has unsafe bindings defined.
How should I go about fixing the dispatch_namespaces error? It's defined on EnvironmentInheritable, so I can either move it to non-inheritable (which shouldn't be breaking as only their union is exported, and if anything this is a fix, or I can manually patch the type to also include dispatch namespaces from the inheritable config (smaller diff, but worse solution imo).

DaniFoldi · 2024-03-05T15:18:35Z

Rebased on main, test failures seemed unrelated. Could you please take another look, @mrbbot?

DaniFoldi requested a review from a team as a code owner February 25, 2024 12:36

dario-piotrowicz approved these changes Feb 25, 2024

View reviewed changes

penalosa reviewed Feb 26, 2024

View reviewed changes

mrbbot approved these changes Mar 4, 2024

View reviewed changes

lrapoport-cf added the awaiting Cloudflare response Awaiting response from workers-sdk maintainer team label Mar 4, 2024

DaniFoldi added 8 commits March 5, 2024 16:09

Add support for new bindings to wrangler types

893bd2c

Actually Workers AI is also Fetcher

f77bb21

Add test

5ffd976

Add changeset

5056a91

Format changelog

4801cd7

Use unknown for AI binding

df77dfa

Make bindings config type validation stricter

1e260ec

Fix typecheck failure as discussed

eed80d3

DaniFoldi force-pushed the wrangler-types-new-bindings branch from ef301a6 to eed80d3 Compare March 5, 2024 15:14

mrbbot merged commit 5b85dc9 into cloudflare:main Mar 6, 2024
17 checks passed

workers-devprod mentioned this pull request Mar 6, 2024

Version Packages #5174

Merged

DaniFoldi mentioned this pull request Aug 5, 2024

🚀 Feature Request: wrangler types to include send_email bindings #4569

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for new bindings to wrangler types #5089

Add support for new bindings to wrangler types #5089

DaniFoldi commented Feb 25, 2024 •

edited by lrapoport-cf

Loading

changeset-bot bot commented Feb 25, 2024 •

edited

Loading

dario-piotrowicz left a comment

dario-piotrowicz Feb 25, 2024

penalosa Feb 26, 2024

DaniFoldi Feb 26, 2024

dario-piotrowicz Feb 26, 2024

DaniFoldi Feb 26, 2024

github-actions bot commented Feb 26, 2024 •

edited

Loading

codecov bot commented Feb 26, 2024 •

edited

Loading

penalosa Feb 26, 2024

DaniFoldi Feb 26, 2024

penalosa Feb 26, 2024

mrbbot commented Mar 4, 2024

DaniFoldi commented Mar 4, 2024

DaniFoldi commented Mar 5, 2024

Add support for new bindings to wrangler types #5089

Add support for new bindings to wrangler types #5089

Conversation

DaniFoldi commented Feb 25, 2024 • edited by lrapoport-cf Loading

changeset-bot bot commented Feb 25, 2024 • edited Loading

🦋 Changeset detected

dario-piotrowicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 26, 2024 • edited Loading

codecov bot commented Feb 26, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrbbot commented Mar 4, 2024

DaniFoldi commented Mar 4, 2024

DaniFoldi commented Mar 5, 2024

DaniFoldi commented Feb 25, 2024 •

edited by lrapoport-cf

Loading

changeset-bot bot commented Feb 25, 2024 •

edited

Loading

github-actions bot commented Feb 26, 2024 •

edited

Loading

codecov bot commented Feb 26, 2024 •

edited

Loading