-
Notifications
You must be signed in to change notification settings - Fork 5.9k
refactor: merge deno_webidl into deno_web
#31561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughWebIDL functionality was consolidated into the deno_web extension. Source imports were updated from ext:deno_webidl/00_webidl.js to ext:deno_web/00_webidl.js across JS/TS files and docs. The deno_webidl extension and its workspace/dependency entries were removed or deprecated; extension dependency lists and initialization calls were updated to omit deno_webidl. TypeScript declaration files were moved/updated to expose the WebIDL API under ext:deno_web, and the core import map was adjusted to point to the new module location. Runtime re-exports referencing deno_webidl were removed. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)ext/web/internal.d.ts (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). (10)
🔇 Additional comments (4)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (68)
Cargo.toml(0 hunks)ext/cache/01_cache.js(1 hunks)ext/cache/lib.rs(1 hunks)ext/canvas/01_image.js(1 hunks)ext/canvas/lib.rs(1 hunks)ext/crypto/00_crypto.js(1 hunks)ext/crypto/README.md(0 hunks)ext/crypto/lib.rs(1 hunks)ext/fetch/20_headers.js(1 hunks)ext/fetch/21_formdata.js(1 hunks)ext/fetch/22_body.js(1 hunks)ext/fetch/23_request.js(1 hunks)ext/fetch/23_response.js(1 hunks)ext/fetch/26_fetch.js(1 hunks)ext/fetch/27_eventsource.js(1 hunks)ext/fetch/README.md(0 hunks)ext/fetch/lib.rs(1 hunks)ext/net/README.md(1 hunks)ext/node/polyfills/worker_threads.ts(1 hunks)ext/web/00_url.js(1 hunks)ext/web/01_broadcast_channel.js(1 hunks)ext/web/01_dom_exception.js(1 hunks)ext/web/01_urlpattern.js(1 hunks)ext/web/02_event.js(1 hunks)ext/web/02_timers.js(1 hunks)ext/web/03_abort_signal.js(1 hunks)ext/web/05_base64.js(1 hunks)ext/web/06_streams.js(1 hunks)ext/web/08_text_encoding.js(1 hunks)ext/web/09_file.js(1 hunks)ext/web/10_filereader.js(1 hunks)ext/web/13_message_port.js(1 hunks)ext/web/14_compression.js(1 hunks)ext/web/15_performance.js(1 hunks)ext/web/16_image_data.js(1 hunks)ext/web/Cargo.toml(1 hunks)ext/web/README.md(0 hunks)ext/web/benches/dict.js(1 hunks)ext/web/benches/dict.rs(1 hunks)ext/web/benches/encoding.rs(0 hunks)ext/web/benches/timers_ops.rs(0 hunks)ext/web/benches/url_ops.rs(0 hunks)ext/web/internal.d.ts(6 hunks)ext/web/lib.rs(1 hunks)ext/web/webtransport.js(1 hunks)ext/webgpu/01_webgpu.js(1 hunks)ext/webgpu/lib.rs(1 hunks)ext/webidl/Cargo.toml(1 hunks)ext/webidl/README.md(1 hunks)ext/webidl/internal.d.ts(0 hunks)ext/webidl/lib.rs(0 hunks)ext/websocket/01_websocket.js(1 hunks)ext/websocket/02_websocketstream.js(1 hunks)ext/websocket/lib.rs(1 hunks)ext/webstorage/lib.rs(1 hunks)runtime/Cargo.toml(0 hunks)runtime/js/11_workers.js(1 hunks)runtime/js/98_global_scope_shared.js(1 hunks)runtime/js/98_global_scope_window.js(1 hunks)runtime/js/98_global_scope_worker.js(1 hunks)runtime/js/99_main.js(1 hunks)runtime/lib.rs(0 hunks)runtime/shared.rs(0 hunks)runtime/snapshot.rs(0 hunks)runtime/snapshot_info.rs(0 hunks)runtime/web_worker.rs(0 hunks)runtime/worker.rs(0 hunks)tools/core_import_map.json(1 hunks)
💤 Files with no reviewable changes (16)
- runtime/worker.rs
- runtime/Cargo.toml
- runtime/snapshot_info.rs
- ext/web/README.md
- ext/webidl/lib.rs
- ext/web/benches/timers_ops.rs
- Cargo.toml
- runtime/lib.rs
- runtime/web_worker.rs
- runtime/snapshot.rs
- ext/web/benches/encoding.rs
- runtime/shared.rs
- ext/fetch/README.md
- ext/crypto/README.md
- ext/web/benches/url_ops.rs
- ext/webidl/internal.d.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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_event.jsruntime/js/99_main.jsext/fetch/26_fetch.jsext/web/13_message_port.jsext/fetch/23_request.jsext/web/01_dom_exception.jsext/webgpu/01_webgpu.jsext/web/16_image_data.jsext/web/00_url.jsruntime/js/11_workers.jsext/web/08_text_encoding.jsruntime/js/98_global_scope_shared.jsext/web/01_urlpattern.jsext/web/03_abort_signal.jsext/web/05_base64.jsext/crypto/00_crypto.jsext/web/02_timers.jsext/fetch/27_eventsource.jsext/fetch/23_response.jsext/web/09_file.jsext/web/10_filereader.jsext/cache/01_cache.jsruntime/js/98_global_scope_window.jsruntime/js/98_global_scope_worker.jsext/web/15_performance.jsext/web/06_streams.jsext/fetch/20_headers.jsext/websocket/01_websocket.jsext/fetch/22_body.jsext/fetch/21_formdata.jsext/websocket/02_websocketstream.jsext/web/14_compression.jsext/web/webtransport.jsext/canvas/01_image.jsext/node/polyfills/worker_threads.tsext/web/01_broadcast_channel.jsext/web/benches/dict.jsext/web/internal.d.ts
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/webgpu/lib.rsext/web/lib.rsext/cache/lib.rsext/fetch/lib.rsext/canvas/lib.rsext/crypto/lib.rsext/websocket/lib.rsext/webstorage/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/webgpu/lib.rsext/web/lib.rsext/cache/lib.rsext/fetch/lib.rsext/canvas/lib.rsext/crypto/lib.rsext/websocket/lib.rsext/web/benches/dict.rsext/webstorage/lib.rs
⚙️ CodeRabbit configuration file
Don't worry about coverage of Rust docstrings. Don't be nitpicky about it. Leave it to the author's judgement if such a documentation is necessary.
Files:
ext/webgpu/lib.rsext/web/lib.rsext/cache/lib.rsext/fetch/lib.rsext/canvas/lib.rsext/crypto/lib.rsext/websocket/lib.rsext/web/benches/dict.rsext/webstorage/lib.rs
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:19:37.808Z
Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
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/webgpu/lib.rsext/web/lib.rsext/fetch/lib.rsext/canvas/lib.rsext/crypto/lib.rstools/core_import_map.jsonext/websocket/lib.rsext/webstorage/lib.rsext/webidl/README.mdext/net/README.md
🧬 Code graph analysis (1)
ext/web/internal.d.ts (2)
ext/web/01_console.js (5)
message(1677-1677)message(1805-1805)message(1822-1822)type(598-598)type(1841-1841)ext/web/13_message_port.js (3)
type(382-382)type(389-389)type(468-468)
⏰ 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). (1)
- GitHub Check: build libs
🔇 Additional comments (54)
ext/web/internal.d.ts (4)
2-2: LGTM!The lint suppression is appropriate for a WebIDL type declarations file that necessarily works with dynamic JavaScript values.
345-500: LGTM!The WebIDL utility functions, factory methods, and type inspection helpers are comprehensive and well-typed. The API surface properly covers dictionary converters, enum converters, nullable/sequence/promise wrappers, branding, and interface conversion utilities.
597-605: LGTM!The inline union type definition for
Transferableis clearer than separate type references.
506-510: LGTM!The formatting changes to inline function parameters and class declarations are minor and don't affect functionality.
Also applies to: 533-537, 559-560, 635-635
ext/web/09_file.js (1)
59-59: LGTM - Import path correctly updated.The migration from
ext:deno_webidltoext:deno_webis straightforward and correct.ext/fetch/21_formdata.js (1)
13-13: LGTM - Import path correctly updated.ext/fetch/20_headers.js (1)
31-31: LGTM - Import path correctly updated.runtime/js/98_global_scope_worker.js (1)
17-17: LGTM - Import path correctly updated.ext/web/03_abort_signal.js (1)
26-26: LGTM - Import path correctly updated.runtime/js/99_main.js (1)
78-78: LGTM - Import path correctly updated.All import migrations from
ext:deno_webidltoext:deno_webare consistent and correct across the reviewed files.ext/web/06_streams.js (1)
93-93: WebIDL import correctly repointed todeno_webImporting
webidlfromext:deno_web/00_webidl.jskeeps all existing usages intact and matches the consolidation goal; no additional changes needed here.ext/web/14_compression.js (1)
20-20: Align compression WebIDL import withdeno_webThe webidl import now comes from
ext:deno_web/00_webidl.js, consistent with the rest of the refactor and without affecting runtime behavior.ext/node/polyfills/worker_threads.ts (1)
26-26: Node worker_threads now consistently source WebIDL fromdeno_webSwitching to
ext:deno_web/00_webidl.jsmatches the new shared WebIDL location; type conversions and runtime semantics remain the same.ext/fetch/22_body.js (1)
40-40: Fetch body WebIDL import updated without behavior change
webidlnow imports fromext:deno_web/00_webidl.js, with all converter and symbol usages unchanged; this is in line with the crate merge.ext/fetch/23_request.js (1)
27-27: Request WebIDL dependency pointed atdeno_webUsing
ext:deno_web/00_webidl.jskeeps all Request-related converters and helpers intact while aligning with the deno_web consolidation.ext/web/benches/dict.js (1)
5-8: Bench now imports WebIDL helpers fromdeno_webThe benchmark’s
convertersandcreateDictionaryConverternow come from the consolidated WebIDL module; no further adjustments needed.ext/fetch/23_response.js (1)
14-14: Response WebIDL import aligned with consolidated moduleImporting
webidlfromext:deno_web/00_webidl.jsis consistent with other fetch components and preserves existing behavior.ext/web/13_message_port.js (1)
34-34: MessagePort WebIDL import now usesdeno_webThe WebIDL utilities for
MessageChannel/MessagePortnow come fromext:deno_web/00_webidl.js, matching the project-wide consolidation and leaving logic intact.ext/web/05_base64.js (1)
16-16: LGTM - Import path updated correctly.The import path change from
ext:deno_webidl/00_webidl.jstoext:deno_web/00_webidl.jsis consistent with the PR's consolidation objective.ext/web/16_image_data.js (1)
4-4: LGTM - Import path updated correctly.runtime/js/98_global_scope_window.js (1)
17-17: LGTM - Import path updated correctly.runtime/js/11_workers.js (1)
23-23: LGTM - Import path updated correctly.runtime/js/98_global_scope_shared.js (1)
30-30: LGTM - Import path updated correctly.ext/web/02_timers.js (1)
16-16: LGTM - Import path updated correctly.ext/web/10_filereader.js (1)
36-36: LGTM - Import path updated correctly.ext/web/lib.rs (1)
112-112: LGTM - WebIDL module correctly consolidated into deno_web extension.The addition of "00_webidl.js" to the deno_web extension completes the consolidation. The old deno_webidl extension is properly deprecated (empty lib.rs, marked in Cargo.toml), and all references in the main codebase have been updated. The only remaining reference to deno_webidl is in ext/web/benches/dict.rs as a local benchmark-only extension, which is appropriate.
ext/canvas/01_image.js (1)
5-5: WebIDL import path migration looks correctImporting WebIDL from
ext:deno_web/00_webidl.jsis consistent with the rest of the file’sdeno_webusage and the PR’s consolidation; no behavior change here.ext/web/15_performance.js (1)
19-19: Performance module now correctly sources WebIDL fromdeno_webThe updated import to
ext:deno_web/00_webidl.jsaligns with the new WebIDL location; existingwebidlusage remains valid.ext/crypto/lib.rs (1)
84-86: Crypto extension dependency now correctly targetsdeno_webSwitching
deno_crypto’s dependency list todeps = [ deno_web ]matches the JS side’s imports and the deprecation ofdeno_webidl; the op set is unchanged.ext/webidl/README.md (1)
3-4: Deprecation notice is clear and sufficientThe README now plainly states that
deno_webidlis deprecated in favor ofdeno_web, which is appropriate for this migration.ext/net/README.md (1)
9-13: Networking README example now matches WebIDL sourceThe sample JS now imports WebIDL from
ext:deno_web/00_webidl.js, consistent with the documenteddeno_webdependency.ext/web/Cargo.toml (1)
36-39: Newdictbench entry is fine; confirm corresponding bench source existsThe added
[[bench]] name = "dict"stanza matches the existing bench pattern. Just make sure there is a correspondingbenches/dict.rs(or explicitpath = ...) socargo bench -p deno_webdoesn’t fail.ext/web/01_dom_exception.js (1)
25-25: DOMException now uses WebIDL fromdeno_webRouting the WebIDL import to
ext:deno_web/00_webidl.jsis consistent with the crate merge; existing branding/configuration logic remains unchanged.ext/crypto/00_crypto.js (1)
90-90: Crypto WebIDL import updated toext:deno_web/00_webidl.jsThe WebIDL import now targets
deno_web, matching the rest of the runtime’s migration away fromdeno_webidl; all subsequentwebidlusage remains valid.ext/web/08_text_encoding.js (1)
46-46: Import path migration toext:deno_web/00_webidl.jslooks correctThe module still uses the same
webidlsurface; only the specifier changed to the new canonical location, which matches the PR’s goal. No further changes needed here.ext/webgpu/lib.rs (1)
65-104: Updateddeno_webgpudependency todeno_webonly is consistentSwitching the extension deps to
[deno_web]matches the new JS imports and ensures00_webidl.jsand related web plumbing load before WebGPU. No functional changes to ops or types here.ext/webgpu/01_webgpu.js (1)
48-48: WebGPU JS now sourcing WebIDL fromdeno_webis aligned with the refactorThe only change is the WebIDL import specifier; all branding and converter usage stays intact and will now go through the unified
deno_webentrypoint. Looks good.ext/webstorage/lib.rs (1)
36-53: Switchingdeno_webstoragedeps fromdeno_webidltodeno_webis safeThis keeps the storage extension wired to the web runtime via
deno_webwhile removing the deprecateddeno_webidldependency. No impact on the Rust ops or storage behavior.ext/canvas/lib.rs (1)
45-50: Canvas extension dependency list correctly dropsdeno_webidlThe extension still depends on
deno_webanddeno_webgpu, which cover its JS needs; removingdeno_webidlhere just reflects the merged WebIDL surface. No behavior change in ops.ext/fetch/lib.rs (1)
147-172:deno_fetchdepending only ondeno_webmatches the new WebIDL wiringRemoving
deno_webidlfrom the deps list is consistent with the JS-side import changes;deno_webremains to provide the Web and WebIDL layer. No issues spotted in the Rust side.ext/cache/lib.rs (1)
100-119: Cache extension now depending ondeno_web+deno_fetchis appropriateThe dependency list reflects actual JS consumers (web + fetch) and removes
deno_webidlas planned. No behavioral changes in cache storage or ops.tools/core_import_map.json (1)
683-683: Core import map now points WebIDL toext:web/00_webidl.jsunderdeno_webNo remaining references to the old
ext:deno_webidl/00_webidl.jsspecifier found in the codebase, so the mapping change is safe.ext/webidl/Cargo.toml (1)
5-11: Deprecation metadata and version bump look consistentVersion bump to
0.223.0and the"DEPRECATED: use deno_web instead"description cleanly signal the transition; no issues from this snippet.ext/fetch/26_fetch.js (1)
41-41: Fetch now correctly sources WebIDL fromdeno_webSwitching the WebIDL import to
"ext:deno_web/00_webidl.js"aligns this module with the new merged extension; usage sites remain unchanged.ext/cache/01_cache.js (1)
21-21: Cache module WebIDL import re-pointed cleanlyThe cache extension now imports WebIDL from
ext:deno_web/00_webidl.jswith no other behavioural changes; the branding/configuration pattern is intact.ext/websocket/01_websocket.js (1)
53-53: WebSocket WebIDL source switched todeno_webImporting WebIDL from
ext:deno_web/00_webidl.jsmatches the new consolidation and keeps the existing converter/branding usage intact.ext/web/00_url.js (1)
36-36: URL module now pulls WebIDL fromdeno_webThe WebIDL import path switch is aligned with the crate merge; URL and URLSearchParams still use the same WebIDL APIs, so behaviour should be preserved.
ext/fetch/27_eventsource.js (1)
23-23: EventSource WebIDL binding migrated todeno_webUsing
ext:deno_web/00_webidl.jshere is consistent with the rest of the refactor and leaves the EventSource implementation unchanged.ext/web/02_event.js (1)
36-36: Core event system now usesdeno_webWebIDL moduleThe updated import to
ext:deno_web/00_webidl.jsmatches the consolidation effort; existing Event/EventTarget machinery keeps the same WebIDL contracts.ext/web/webtransport.js (1)
4-4: WebTransport now bound todeno_webWebIDLRepointing the WebIDL import to
ext:deno_web/00_webidl.jsis consistent with the new architecture; the extensive converter and branding usage in this file remains unchanged.ext/web/01_broadcast_channel.js (1)
22-22: WebIDL import path change matches the deno_web consolidationSwitching to
ext:deno_web/00_webidl.jsis consistent with the crate merge and keeps the runtime behavior intact since only the source module moved.ext/websocket/02_websocketstream.js (1)
35-35: WebSocketStream WebIDL binding correctly switched to deno_webUsing
ext:deno_web/00_webidl.jshere aligns with the rest of the migration and should keep allwebidl.convertersand branding semantics unchanged.ext/web/01_urlpattern.js (1)
30-30: URLPattern now sourced from deno_web WebIDL moduleThe import swap to
ext:deno_web/00_webidl.jsis consistent with the shared WebIDL consolidation and does not affect the URLPattern behavior.ext/websocket/lib.rs (1)
848-868: Removing deno_webidl from websocket extension deps is appropriateDepending only on
deno_webnow that it provides the WebIDL surface keeps the extension’s dependency set minimal while preserving the same ops and ESM modules.Based on learnings, this keeps the extension wiring lean while still exposing the required ops from
ext/websocket/lib.rs.
ext/web/internal.d.ts
Outdated
| messageData: messagePort.MessageData | ||
| ): [object, object[]]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file around the relevant lines
cat -n ext/web/internal.d.ts | sed -n '600,620p'Repository: denoland/deno
Length of output: 755
🏁 Script executed:
# Search for messagePort references and declarations
rg -n "messagePort|MessageData" ext/web/internal.d.tsRepository: denoland/deno
Length of output: 205
🏁 Script executed:
# Search the entire codebase for messagePort namespace/module declarations
rg -n "namespace messagePort|module messagePort|import.*messagePort" --type=tsRepository: denoland/deno
Length of output: 39
Change messagePort.MessageData to MessageData on line 614.
The MessageData interface is defined locally in this module (lines 606-609), not in a messagePort namespace. The reference should be just MessageData.
🤖 Prompt for AI Agents
In ext/web/internal.d.ts around lines 614-615, the parameter type is incorrectly
referenced as messagePort.MessageData while the MessageData interface is defined
locally in this module; change the type to MessageData (remove the messagePort.
prefix) so the signature uses the locally defined interface.
|
Please run |
This commit deprecated
deno_webidlcrate and merges it intodeno_web.This will allow us to limit number of crates we need to publish and (maybe)
improve compile and link times.
The actual
ext/webidldirectory will be removed in a follow up PR, once a newversion is published and points to
deno_webcrate.