feat(web): update QuotaExceededError to spec #31400
Conversation
This commit refactors `QuotaExceededError` to be a proper subclass of `DOMException`, aligning with the WHATWG WebIDL update. Previously, `QuotaExceededError` was a `DOMException` instance with a specific name.
The changes involve:
- Removing legacy `QuotaExceededError` handling from `ext/web/01_dom_exception.js`.
- Introducing a new `QuotaExceededError` class in `ext/web/02_quota_exceeded_error.js` that extends `DOMException`.
- Updating `ext/web/lib.rs` to include the new JavaScript file in the build.
- Exposing `QuotaExceededError` globally in `runtime/js/98_global_scope_shared.js`.
- Modifying the error builder in `runtime/js/99_main.js` to instantiate the new `QuotaExceededError` class.
- Updating TypeScript definitions in `cli/tsc/dts/lib.deno_web.d.ts` to reflect the new class.
This ensures `QuotaExceededError` has the correct prototype chain and improves adherence to web standards. Existing `new DOMException("message", "QuotaExceededError")` calls will still function, but `code` will be 0 as it's no longer mapped directly in DOMException.
|
|
WalkthroughAdds a new QuotaExceededError JS class (subclassing DOMException), registers it with webidl, and exposes it on the global scope. Removes the Sequence Diagram(s)mermaid Note over WebStorage,Rust: storage operation that may exceed quota mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cli/tsc/dts/lib.deno_web.d.ts(1 hunks)ext/web/01_dom_exception.js(0 hunks)ext/web/02_quota_exceeded_error.js(1 hunks)ext/web/lib.rs(1 hunks)runtime/js/98_global_scope_shared.js(2 hunks)runtime/js/99_main.js(2 hunks)
💤 Files with no reviewable changes (1)
- ext/web/01_dom_exception.js
🧰 Additional context used
📓 Path-based instructions (3)
ext/**/lib.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Extensions should provide ops (operations) exposed to JavaScript in Rust code within
ext/<extension_name>/directories
Files:
ext/web/lib.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: For debugging Rust code, set breakpoints in IDE debuggers (VS Code with rust-analyzer, IntelliJ IDEA) or uselldbdirectly
Useeprintln!()ordbg!()macros for debug prints in Rust code
Files:
ext/web/lib.rs
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: For JavaScript runtime debugging, enable V8 inspector with--inspect-brkflag and connect Chrome DevTools tochrome://inspect
Useconsole.log()for debug prints in JavaScript runtime code
Files:
runtime/js/99_main.jsext/web/02_quota_exceeded_error.jsruntime/js/98_global_scope_shared.jscli/tsc/dts/lib.deno_web.d.ts
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:19:37.798Z
Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.798Z
Learning: Applies to ext/**/lib.rs : Extensions should provide ops (operations) exposed to JavaScript in Rust code within `ext/<extension_name>/` directories
Applied to files:
ext/web/lib.rs
🧬 Code graph analysis (1)
ext/web/02_quota_exceeded_error.js (1)
ext/web/01_dom_exception.js (4)
primordials(11-23)_name(28-28)_message(29-29)_code(30-30)
🪛 Biome (2.1.2)
ext/web/02_quota_exceeded_error.js
[error] 13-13: Do not shadow the global "Symbol" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 GitHub Check: lint debug macos-x86_64
ext/web/02_quota_exceeded_error.js
[failure] 23-23:
�[0m�[1moptions is never used
[failure] 46-46:
�[0m�[1mUse null [[prototype]] object in the define property
[failure] 23-23:
�[0m�[1mUse null [[prototype]] object in the default parameter
🪛 GitHub Check: lint debug windows-x86_64
ext/web/02_quota_exceeded_error.js
[failure] 23-23:
�[0m�[1moptions is never used
[failure] 46-46:
�[0m�[1mUse null [[prototype]] object in the define property
[failure] 23-23:
�[0m�[1mUse null [[prototype]] object in the default parameter
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: test debug macos-x86_64
- GitHub Check: test release linux-x86_64
- GitHub Check: test debug windows-x86_64
- GitHub Check: test debug linux-aarch64
- GitHub Check: test debug linux-x86_64
- GitHub Check: test debug macos-aarch64
- GitHub Check: build libs
🔇 Additional comments (4)
ext/web/lib.rs (1)
112-120: ESM registration of02_quota_exceeded_error.jslooks correctAdding the module to the
esmarray right after01_dom_exception.jscleanly wires the newQuotaExceededErrorimplementation into the web extension; no additional Rust ops are needed here.runtime/js/99_main.js (1)
79-81: Switching the quota builder toQuotaExceededErroris soundImporting
QuotaExceededErrorand havingDOMExceptionQuotaExceededErrorreturnnew QuotaExceededError(msg)preserves backwards compatibility (instanceof DOMExceptionstill holds) while exposing the more specific subclass. The wiring throughcore.registerErrorBuilderremains unchanged.Also applies to: 352-355
runtime/js/98_global_scope_shared.js (1)
30-32: Global exposure ofQuotaExceededErroris consistent with other Web APIsImporting
QuotaExceededErrorand exposing it onwindowOrWorkerGlobalScopeviacore.propNonEnumerablematches the pattern used forDOMExceptionand other platform classes; this will makeQuotaExceededErroravailable on both window and worker globals as expected.Also applies to: 68-89
cli/tsc/dts/lib.deno_web.d.ts (1)
70-73: Type declaration forQuotaExceededErrormatches the runtime shapeDeclaring
QuotaExceededErroras aDOMExceptionsubclass with(message?: string, options?: ExceptionInformation)aligns with the new JS implementation and makes the new class part of the publicdeno_websurface.
ext/web/02_quota_exceeded_error.js
Outdated
| import { primordials } from "ext:core/mod.js"; | ||
| const { | ||
| ObjectDefineProperty, | ||
| Symbol, | ||
| } = primordials; | ||
| import * as webidl from "ext:deno_webidl/00_webidl.js"; | ||
| import { DOMException } from "ext:deno_web/01_dom_exception.js"; | ||
|
|
||
| const _name = Symbol("[[name]]"); | ||
| const _message = Symbol("[[message]]"); | ||
| const _code = Symbol("[[code]]"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix lints in QuotaExceededError (Symbol shadowing, unused options, null-proto objects)
The overall implementation of QuotaExceededError as a DOMException subclass looks fine, but the current file will trip the linters:
- Biome flags shadowing the global
Symbol. - Lint reports the
optionsparameter as unused. - Lint requires null-
[[Prototype]]objects for theoptionsdefault and theObjectDefinePropertydescriptor.
You can address all three without changing observable behavior by applying a small refactor like this:
import { primordials } from "ext:core/mod.js";
const {
- ObjectDefineProperty,
- Symbol,
+ ObjectDefineProperty,
+ Symbol: PrimordialsSymbol,
} = primordials;
import * as webidl from "ext:deno_webidl/00_webidl.js";
import { DOMException } from "ext:deno_web/01_dom_exception.js";
-const _name = Symbol("[[name]]");
-const _message = Symbol("[[message]]");
-const _code = Symbol("[[code]]");
+const _name = PrimordialsSymbol("[[name]]");
+const _message = PrimordialsSymbol("[[message]]");
+const _code = PrimordialsSymbol("[[code]]");
class QuotaExceededError extends DOMException {
- constructor(message = "", options = {}) {
+ constructor(message = "", options = { __proto__: null }) {
super(message, "QuotaExceededError");
+ // `options` is currently reserved for spec-aligned extensions; mark it as
+ // used so linters don’t treat it as dead.
+ void options;
this[webidl.brand] = webidl.brand;
this[_name] = "QuotaExceededError";
this[_message] = message;
this[_code] = 22;
}
@@
-ObjectDefineProperty(globalThis, "QuotaExceededError", {
- value: QuotaExceededError,
- writable: true,
- enumerable: false,
- configurable: true,
-});
+ObjectDefineProperty(globalThis, "QuotaExceededError", {
+ __proto__: null,
+ value: QuotaExceededError,
+ writable: true,
+ enumerable: false,
+ configurable: true,
+});This keeps the runtime behavior the same while satisfying the static analysis checks and following the project’s null-prototype object conventions.
Also applies to: 22-24, 44-51
🧰 Tools
🪛 Biome (2.1.2)
[error] 13-13: Do not shadow the global "Symbol" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
In ext/web/02_quota_exceeded_error.js around lines 10–20 (also apply same
pattern to 22–24 and 44–51): avoid shadowing the global Symbol by aliasing the
primordials export (e.g., extract Symbol as PrimordialsSymbol and use that alias
when creating internal Symbols), stop reporting the options parameter as unused
by using a null-prototype default options object (e.g., const defaultOptions =
Object.create(null) and use that instead of undefined), and ensure any
property-descriptor objects passed to ObjectDefineProperty are null-prototype
objects (create the descriptor via Object.create(null) and set
configurable/enumerable/writable/value fields there) so linters are satisfied
without changing runtime behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ext/web/02_quota_exceeded_error.js (1)
1-2: Remove this obsolete module instead of leaving a stub.If QuotaExceededError is now fully handled in
01_dom_exception.js, this file should be deleted and any references to02_quota_exceeded_error.jsinext/web/lib.rs(or elsewhere) should be removed. Keeping a comment‑only stub is noisy and can confuse future readers about where the real implementation lives.tests/unit/webstorage_test.ts (1)
15-42: Tighten assertions to expectQuotaExceededErrorsubclass, not justDOMException.The current tests are correct: they verify a
DOMExceptionis thrown anderr.name === "QuotaExceededError"for all quota‑exceeded cases. To better exercise the new subclass behavior introduced in this PR, consider expecting theQuotaExceededErrortype directly:- let err = assertThrows( + let err = assertThrows( () => { localStorage.setItem("k", "v".repeat(15 * 1024 * 1024)); }, - DOMException, + QuotaExceededError, ); assertEquals(err.name, "QuotaExceededError"); @@ - err = assertThrows( + err = assertThrows( () => { localStorage.setItem("k".repeat(15 * 1024 * 1024), "v"); }, - DOMException, + QuotaExceededError, ); @@ - err = assertThrows( + err = assertThrows( () => { localStorage.setItem( "k".repeat(5 * 1024 * 1024), "v".repeat(5 * 1024 * 1024), ); }, - DOMException, + QuotaExceededError, );This will fail if we ever regress to throwing a plain
DOMExceptionwith that name instead of the dedicatedQuotaExceededErrorsubclass.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ext/web/01_dom_exception.js(1 hunks)ext/web/02_quota_exceeded_error.js(1 hunks)tests/unit/webstorage_test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ext/web/01_dom_exception.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: For JavaScript runtime debugging, enable V8 inspector with--inspect-brkflag and connect Chrome DevTools tochrome://inspect
Useconsole.log()for debug prints in JavaScript runtime code
Files:
ext/web/02_quota_exceeded_error.jstests/unit/webstorage_test.ts
🧬 Code graph analysis (1)
tests/unit/webstorage_test.ts (2)
runtime/js/99_main.js (1)
err(291-291)tests/unit/test_util.ts (1)
assertThrows(19-19)
Remove incorrectly added and its reference from . is an instance of with a specific name, not a separate class. This fixes CI failures related to Web API error handling and snapshot generation.
| core.registerErrorBuilder( | ||
| "DOMExceptionQuotaExceededError", | ||
| function DOMExceptionQuotaExceededError(msg) { | ||
| return new DOMException(msg, "QuotaExceededError"); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
It needs to be registered here to be accessible from Rust's #[class("QuotaExceededError")] macro.
| NetworkError: { value: NETWORK_ERR }, | ||
| AbortError: { value: ABORT_ERR }, | ||
| URLMismatchError: { value: URL_MISMATCH_ERR }, | ||
| QuotaExceededError: { value: QUOTA_EXCEEDED_ERR }, |
There was a problem hiding this comment.
Need to stay for compat.
bartlomieju
changed the title
3 participants
Closes #30028
This PR aligns Deno with the WHATWG WebIDL update (whatwg/webidl#1465), which changes QuotaExceededError from a named DOMException entry to a dedicated subclass.
Overview
QuotaExceededError is now implemented as a proper DOMException subclass instead of relying on the legacy "QuotaExceededError" name. This brings the behavior in line with modern web standards while keeping backward compatibility.
Changes
Why :
The spec allows two paths :