feat: add Artifacts binding TypeScript definitions#6508
feat: add Artifacts binding TypeScript definitions#6508dario-piotrowicz merged 16 commits intocloudflare:mainfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
7071b12 to
23898be
Compare
23898be to
00d8b17
Compare
Merging this PR will not alter performance
Comparing Footnotes
|
b763d83 to
daeaa28
Compare
5eb297f to
56e4a11
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6508 +/- ##
=======================================
Coverage 70.86% 70.86%
=======================================
Files 438 438
Lines 123696 123696
Branches 19467 19467
=======================================
Hits 87660 87660
Misses 24510 24510
Partials 11526 11526 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d0d1645 to
2b42cb6
Compare
|
/bonk review this pr |
There was a problem hiding this comment.
Review Summary
The type definitions are well-documented with solid JSDoc comments, and the naming conventions (Artifacts* prefix) follow the codebase standard. There are several issues to address before this can merge:
generated-snapshot/files should not be manually edited — they must be produced byjust generate-types. The hand-edited snapshots will diverge from the pipeline output and CI will likely reject them.ArtifactsRepoduplicates all fields fromArtifactsRepoInfoinstead of extending it — this creates a maintenance burden.scopefield inArtifactsCreateTokenResultshould use a string literal union to matchArtifactsTokenInfo.scopeandcreateToken()'s parameter type.get()return type should bePromise<ArtifactsRepo | null>since the JSDoc says it returns null when not found.import()target.optsnesting is unnecessarily deep — consider flattening.- Missing type tests — other bindings (D1, AI, RPC, etc.) have type tests in
types/test/types/.
This review was generated by an AI assistant and may contain inaccuracies. Please verify all suggestions before applying.
Add Artifacts binding types to the generated snapshot files (index.d.ts and index.ts) to match bazel-generated output.
- fork(): flatten to (name, opts?) with description, readOnly, defaultBranchOnly
- create(): add description and setDefaultBranch to opts
- import(): restructure to params object with source and target
- Flatten return types: remove '& { repo: ArtifactsRepo }' intersections
- get(): add JSDoc note about transient states (import/fork in progress)
- Update all 4 generated snapshot files to match
- Remove Artifacts.import() method entirely
- Add ArtifactsGetRepoResult type with repo handle and status field
- get() now returns { repo, status } where status is 'ready' | 'importing' | 'forking'
- Update all generated snapshots
ArtifactsGetRepoResult is now:
| { status: 'ready'; repo: ArtifactsRepo }
| { status: 'not_found' }
| { status: 'importing'; retryAfter: number }
| { status: 'forking'; retryAfter: number }
- Rename expiresAt → tokenExpiresAt on ArtifactsCreateRepoResult - Remove ArtifactsTokenValidation and validateToken() - Remove ArtifactsGetRepoResult union, get() returns ArtifactsRepo directly - Inline repo fields directly on ArtifactsRepo (no more info() method) - Restore import() with source/target params structure - Update all generated snapshots
Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
6ae35d7 to
fca1d4c
Compare
fca1d4c to
0996d4f
Compare
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
- create() returns ArtifactsCreateRepoResult (not & { repo })
- get() returns ArtifactsRepo (throws on not found, no null)
- Remove info() — repo metadata is on the handle directly
(ArtifactsRepo extends ArtifactsRepoInfo)
- Remove validateToken() — not in workerd types or binding
- Add import() — was missing from docs entirely
- Fix expiresAt → tokenExpiresAt in get-started example
- Add namespace note (any name works, not just 'default')
- Remove null checks on get() — it throws, not returns null
Aligned with: cloudflare/workerd#6508
Summary
Add TypeScript type definitions for the Artifacts binding
Changes
New file:
types/defines/artifacts.d.ts