refactor(web): use singular requestFileHandle() instead of requestFileHandles()#19780
refactor(web): use singular requestFileHandle() instead of requestFileHandles()#19780tomayac wants to merge 2 commits into
Conversation
…les() Switch to the new singular API as adopted in: WICG/cross-origin-storage#61
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for OPFSAccessMode in the artifact cache, updates the CrossOriginStorage to use a singular requestFileHandle API, and adds persistent caching of hash entries using the Cache API. It also refactors ArtifactOPFSCache to support reading arraybuffer types and avoids cloning responses when writing to the store. However, the review comments point out critical TypeScript compilation errors: the type OPFSAccessMode and the updated constructor signature for OPFSStore (which now accepts accessMode) are missing from web/src/opfs_store.ts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| import { OPFSStore, type OPFSAccessMode } from "./opfs_store"; | ||
|
|
||
| export type { OPFSAccessMode } from "./opfs_store"; |
There was a problem hiding this comment.
The type OPFSAccessMode is imported and exported from ./opfs_store, but it is not defined or exported in web/src/opfs_store.ts. This will cause a TypeScript compilation error.
There was a problem hiding this comment.
Thanks for the review! These are pre-existing in main — OPFSAccessMode and the accessMode constructor parameter were introduced in web/src/opfs_store.ts before this PR was opened and are already present on main. This PR only touches the COS-specific lines (requestFileHandle rename).
| constructor(scope: string, accessMode: OPFSAccessMode = "async") { | ||
| this.store = new OPFSStore(scope, accessMode); | ||
| } |
There was a problem hiding this comment.
The OPFSStore constructor in web/src/opfs_store.ts only accepts a single parameter scope: string. Passing accessMode as a second argument will result in a TypeScript compilation error. If accessMode support is intended, web/src/opfs_store.ts needs to be updated to support it.
There was a problem hiding this comment.
Thanks for the review! These are pre-existing in main — OPFSAccessMode and the accessMode constructor parameter were introduced in web/src/opfs_store.ts before this PR was opened and are already present on main. This PR only touches the COS-specific lines (requestFileHandle rename).
|
Hi @tomayac thanks for the contribution. Please take a look at the gemini review and I think some of them are helpful. |
|
Thank you. Checked both comments and responded. Could you please take another look? Thanks in advance! |
guan404ming
left a comment
guan404ming
left a comment
There was a problem hiding this comment.
CI fails on the PR-body lint: it rejects @-mentions. Please remove the @ from the names in the description, rebase and push. I think then the CI will pass. Code LGTM.
2 participants
Switches from the deprecated plural
requestFileHandles([hash])to the new singularrequestFileHandle(hash)API, which returns a handle directly rather than a single-element array. Also updates the interface definition and removes the now-redundanthandles[0]indexing.The rename was adopted in the COS spec after a survey of all known real-world implementations found that every call site passed a single-element array and immediately indexed
[0]— no implementation ever used the plural form as a batch.FYI guan404ming CharlieFRuan — as reviewers of the original COS PR (#18893).
See WICG/cross-origin-storage#61 for details.