Skip to content

Conversation

@stroiman
Copy link
Member

No description provided.

No longer necessary when the Window owns the clock
Remove "safe" versions of functions. Can be added by a simple wrapper if
relevant; but adds complexity to the interface
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 13, 2026

Walkthrough

This changeset replaces SafeTaskCallback with TaskCallback across the timing APIs, introduces clock.TaskHandle in place of int for timer handles, adds QueueMacrotask, and removes the Clock() accessor from script context types. Codegen was updated to carry operation ReturnType via a GoType/GoTypeGenerator flow and to emit TaskHandle decoders/encoders. Multiple call sites, generated bindings, and tests were adapted to the new TaskCallback/TaskHandle APIs.

Possibly related PRs

  • gost-dom/browser PR 219 — Updates timer/task scheduling types and WindowOrWorkerGlobalScope timer method signatures to use TaskCallback and TaskHandle.
  • gost-dom/browser PR 243 — Modifies window/global-scope timeout APIs and their generated bindings (setTimeout/setInterval/clear*), aligning with TaskHandle usage.
  • gost-dom/browser PR 242 — Rewires how ScriptContext exposes the Clock and adjusts clock wiring that overlaps with removal of the Clock() accessor.
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether any description exists that relates to the changeset. Add a pull request description that explains the purpose and scope of the clock refactoring, including key API changes and their rationale.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Refactor/decouple clock' is clearly related to the main changes in the PR, which comprehensively refactor the clock system and decouple its API from SafeTaskCallback.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42c811e and 23d0a40.

📒 Files selected for processing (20)
  • html/window.go
  • html/window_or_worker_global_scope.go
  • internal/clock/clock.go
  • internal/clock/clock_test.go
  • internal/code-gen/customrules/customrules.go
  • internal/code-gen/customrules/go_type.go
  • internal/code-gen/customrules/html_rules.go
  • internal/code-gen/gotypes/gotypes.go
  • internal/code-gen/interfaces/idl_interface.go
  • internal/code-gen/packagenames/packagenames.go
  • internal/code-gen/scripting/codecs.go
  • internal/code-gen/scripting/op_callback_methods.go
  • internal/html/xhr/xml_http_request.go
  • internal/interfaces/html-interfaces/timeout.go
  • internal/interfaces/html-interfaces/window_or_worker_global_scope_generated.go
  • scripting/internal/html/initializer.go
  • scripting/internal/html/window_or_worker_global_scope.go
  • scripting/internal/html/window_or_worker_global_scope_generated.go
  • scripting/sobekengine/script_context.go
  • scripting/v8engine/script_context.go
💤 Files with no reviewable changes (3)
  • scripting/v8engine/script_context.go
  • html/window.go
  • scripting/sobekengine/script_context.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-30T14:56:35.529Z
Learnt from: stroiman
Repo: gost-dom/browser PR: 115
File: scripting/internal/streams/readable_stream_byobreader_generated.go:0-0
Timestamp: 2025-06-30T14:56:35.529Z
Learning: In the gost-dom/browser codebase, files with the `_generated.go` suffix are generated files that often have companion files without that suffix containing additional methods. When reviewing generated files, always check for companion files that may contain missing method implementations before flagging them as issues.

Applied to files:

  • internal/code-gen/customrules/go_type.go
🧬 Code graph analysis (16)
internal/code-gen/customrules/go_type.go (2)
internal/code-gen/gotypes/gotypes.go (1)
  • GoType (12-16)
internal/code-gen/gen/variable/var.go (2)
  • Type (41-43)
  • Name (38-40)
internal/code-gen/customrules/html_rules.go (3)
internal/code-gen/gotypes/gotypes.go (3)
  • TimeDuration (20-23)
  • TaskHandle (24-27)
  • GoType (12-16)
internal/code-gen/customrules/customrules.go (1)
  • ArgumentRules (115-115)
internal/code-gen/customrules/go_type.go (1)
  • GoType (9-9)
internal/interfaces/html-interfaces/timeout.go (1)
internal/clock/clock.go (1)
  • TaskCallback (24-24)
internal/interfaces/html-interfaces/window_or_worker_global_scope_generated.go (2)
internal/interfaces/html-interfaces/timeout.go (1)
  • TimerHandler (5-5)
internal/code-gen/gotypes/gotypes.go (1)
  • TaskHandle (24-27)
internal/code-gen/interfaces/idl_interface.go (1)
internal/code-gen/customrules/go_type.go (1)
  • GoTypeGenerator (11-11)
internal/code-gen/scripting/codecs.go (1)
internal/code-gen/gotypes/gotypes.go (1)
  • TaskHandle (24-27)
internal/code-gen/scripting/op_callback_methods.go (3)
internal/code-gen/scripting/return_value_generator.go (1)
  • ReturnValueGenerator (9-14)
internal/code-gen/customrules/go_type.go (1)
  • GoType (9-9)
internal/code-gen/gotypes/gotypes.go (1)
  • GoType (12-16)
internal/code-gen/customrules/customrules.go (2)
internal/code-gen/customrules/go_type.go (1)
  • GoType (9-9)
internal/code-gen/gotypes/gotypes.go (1)
  • GoType (12-16)
scripting/internal/html/initializer.go (2)
internal/clock/clock.go (1)
  • Clock (72-100)
scripting/internal/dom/errors.go (1)
  • HandleJSCallbackError (13-19)
scripting/internal/html/window_or_worker_global_scope_generated.go (3)
internal/interfaces/html-interfaces/window_or_worker_global_scope_generated.go (1)
  • WindowOrWorkerGlobalScope (10-16)
scripting/internal/js/arguments.go (1)
  • ConsumeArgument (21-49)
scripting/internal/codec/decoders.go (1)
  • ZeroValue (15-15)
internal/html/xhr/xml_http_request.go (1)
internal/clock/clock.go (1)
  • SafeTask (28-28)
html/window_or_worker_global_scope.go (2)
internal/clock/clock.go (2)
  • TaskCallback (24-24)
  • TaskHandle (21-21)
internal/code-gen/gotypes/gotypes.go (1)
  • TaskHandle (24-27)
scripting/internal/html/window_or_worker_global_scope.go (6)
scripting/internal/dom/errors.go (1)
  • HandleJSCallbackError (13-19)
scripting/internal/js/callback_context.go (1)
  • Scope (29-37)
internal/code-gen/gotypes/gotypes.go (1)
  • TaskHandle (24-27)
internal/clock/clock.go (1)
  • TaskHandle (21-21)
scripting/internal/codec/decoders.go (1)
  • DecodeInt (35-37)
scripting/internal/codec/encoders.go (1)
  • EncodeInt (71-73)
internal/clock/clock.go (2)
internal/code-gen/packagenames/packagenames.go (1)
  • Clock (17-17)
html/window.go (1)
  • Clock (61-85)
internal/code-gen/gotypes/gotypes.go (2)
internal/code-gen/customrules/go_type.go (1)
  • GoType (9-9)
internal/code-gen/packagenames/packagenames.go (1)
  • Clock (17-17)
internal/clock/clock_test.go (1)
internal/clock/clock.go (3)
  • TaskCallback (24-24)
  • New (112-123)
  • OfIsoString (442-448)
⏰ 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). (4)
  • GitHub Check: Tests on linux arm64 with Go
  • GitHub Check: Tests on linux amd64 with Go
  • GitHub Check: Tests on darwin arm64 with Go
  • GitHub Check: Tests on darwin amd64 with Go
🔇 Additional comments (21)
internal/code-gen/customrules/html_rules.go (2)

3-9: LGTM! Clean aliasing of GoType values.

The local aliases timeDuration and taskHandle provide readable shorthand for the gotypes constants. This approach keeps the rule definitions concise while maintaining type safety.


47-62: LGTM! Consistent operation rules for timer APIs.

The setTimeout/setInterval operations correctly specify:

  • ReturnType: taskHandle to generate clock.TaskHandle return type
  • GoType: timeDuration for the timeout argument

The clearTimeout/clearInterval operations correctly accept GoType: taskHandle for the id argument, maintaining consistency with the return types of the corresponding set functions.

internal/code-gen/packagenames/packagenames.go (1)

17-17: LGTM! New Clock package constant.

The Clock constant follows the established naming pattern and is correctly positioned within the const block. This enables type-safe references to the clock package path throughout the code generation infrastructure.

internal/clock/clock.go (3)

26-28: LGTM! Clean helper for error-free callbacks.

SafeTask provides a concise way to adapt simple func() callbacks to the TaskCallback signature. This replaces the removed SafeTaskCallback type with a more composable function-based approach.


376-387: LGTM! Unified callback type for SetInterval.

Changing SetInterval to accept TaskCallback directly aligns it with SetTimeout, creating a consistent API surface. Callers that previously used SafeTaskCallback can now use SafeTask(fn) for the same effect.


403-406: LGTM! EnqueueMacrotask provides clear semantics.

Delegating to SetTimeout(task, 0) is the correct implementation for macrotask queuing - it places the task at the end of the current macrotask queue while respecting task ordering.

internal/interfaces/html-interfaces/timeout.go (1)

5-6: LGTM! Type aliases updated to TaskCallback.

Both TimerHandler and VoidFunction now alias clock.TaskCallback, aligning with the PR's goal of unifying callback types. The separate aliases are appropriate as they represent distinct WebIDL concepts even though they share the same underlying Go type.

internal/clock/clock_test.go (3)

101-112: LGTM! Error propagation test validates TaskCallback behavior.

The test correctly demonstrates that TaskCallback functions can return errors (line 103) while wrapTask-wrapped functions return nil. This validates the error handling semantics of the unified callback type.


139-145: LGTM! Tick test validates microtask/macrotask ordering.

The test correctly uses EnqueueMacrotask for the task queue and AddMicrotask for the microtask queue, validating that microtasks execute before macrotasks as per the event loop specification.


198-216: LGTM! Event processing tests updated to TaskCallback signature.

The AddEvent calls correctly use the new func() error signature, returning nil for success cases. The test structure properly validates async event processing behavior.

internal/code-gen/customrules/customrules.go (1)

95-106: LGTM! ReturnType field enables custom return type generation.

The new ReturnType GoType field allows operations to specify their return type explicitly, which is essential for the setTimeout/setInterval functions to return clock.TaskHandle instead of the default IDL-derived type. The zero value ensures backward compatibility with existing rules.

internal/code-gen/scripting/codecs.go (1)

52-54: decodeTaskHandle is properly defined in scripting/internal/html/window_or_worker_global_scope.go and the reference in the generated code is valid.

internal/code-gen/scripting/op_callback_methods.go (1)

53-59: LGTM!

The addition of rules.ReturnType to ReturnValueGenerator correctly wires custom Go types into the generated callback methods. The map lookup safely returns a zero value for operations without custom rules, which is properly handled downstream.

scripting/internal/html/window_or_worker_global_scope_generated.go (1)

33-33: LGTM!

The generated code correctly switches from int-based encoding/decoding to TaskHandle-based helpers (encodeTaskHandle/decodeTaskHandle), aligning with the updated WindowOrWorkerGlobalScope interface that now uses clock.TaskHandle for timeout/interval methods.

Also applies to: 41-41, 62-62, 70-70

internal/code-gen/customrules/go_type.go (1)

3-24: LGTM! Clean implementation of the code generator pattern.

The type alias approach avoids circular imports while the GoTypeGenerator properly implements the generator interface. The logic correctly handles local types, package-qualified types, and pointer types.

scripting/internal/html/window_or_worker_global_scope.go (1)

18-23: Callback error handling pattern looks correct.

The callback signature change to func() error aligns with clock.TaskCallback. Errors from JS callbacks are properly handled via HandleJSCallbackError which logs and dispatches error events, so returning nil here is intentional.

internal/interfaces/html-interfaces/window_or_worker_global_scope_generated.go (1)

10-16: Interface signature updates look correct.

The timer-related methods consistently use clock.TaskHandle for handle parameters and return values, aligning with the PR's objective to decouple the clock system.

internal/code-gen/gotypes/gotypes.go (2)

7-18: Well-documented struct definition with clean zero-value check.

The GoType struct is clearly documented, and the Zero() method using value comparison with an empty struct literal is idiomatic Go.


24-27: TaskHandle type definition looks correct.

The new TaskHandle GoType references packagenames.Clock which maps to the internal clock package path.

html/window_or_worker_global_scope.go (2)

20-33: LGTM! ClearTimeout, SetInterval, and ClearInterval are implemented correctly.

These methods properly use clock.TaskHandle and delegate to the clock's Cancel and SetInterval methods without losing error information.


35-37: LGTM! QueueMicrotask correctly passes the callback.

The task is passed directly to AddMicrotask without wrapping, preserving error semantics.

Comment on lines 228 to 230
if !o.Rules.ReturnType.Zero() {
return g.List(customrules.GoTypeGenerator(o.Rules.ReturnType))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the file structure and understand the context better
head -250 internal/code-gen/interfaces/idl_interface.go | tail -50

Repository: gost-dom/browser

Length of output: 1335


🏁 Script executed:

# Search for usage patterns combining ReturnType and HasError
rg "ReturnType|HasError" internal/code-gen/interfaces/idl_interface.go -n -A 2 -B 2 | head -100

Repository: gost-dom/browser

Length of output: 842


🏁 Script executed:

# Look for test files that might show expected behavior
fd -e test.go -e _test.go | xargs rg -l "ReturnType\|HasError" 2>/dev/null | head -10

Repository: gost-dom/browser

Length of output: 42


🏁 Script executed:

# Search for GoTypeGenerator usage to understand what it does
rg "GoTypeGenerator" --type go -B 3 -A 3

Repository: gost-dom/browser

Length of output: 1713


🏁 Script executed:

# Look for any documentation or comments about custom ReturnType
rg "ReturnType" --type go -B 5 -A 1 | head -150

Repository: gost-dom/browser

Length of output: 6613


🏁 Script executed:

# Search for actual operation rules that set both ReturnType and HasError
rg "ReturnType.*HasError|HasError.*ReturnType" --type go -B 3 -A 3

Repository: gost-dom/browser

Length of output: 42


🏁 Script executed:

# Look for YAML/JSON config files that might show ReturnType usage
fd -e yaml -e yml -e json | xargs rg "ReturnType" 2>/dev/null | head -20

Repository: gost-dom/browser

Length of output: 42


🏁 Script executed:

# Check test cases or examples for operation rules
fd -e go | xargs rg "OperationRule|ReturnType" 2>/dev/null | grep -i test | head -30

Repository: gost-dom/browser

Length of output: 42


🏁 Script executed:

# Search for any explicit design documentation about custom ReturnType vs HasError
rg "custom.*[Rr]eturn|[Rr]eturn.*custom" --type go -B 2 -A 2 | head -50

Repository: gost-dom/browser

Length of output: 3468


🏁 Script executed:

# Check the OperationRule struct definition and its documentation
rg -A 15 "type OperationRule struct" internal/code-gen/customrules/

Repository: gost-dom/browser

Length of output: 1238


🏁 Script executed:

# Search for any comments or TODOs about this design choice
rg "ReturnType|HasError" --type go -B 3 -A 3 | rg -i "todo|fixme|note|design|intentional"

Repository: gost-dom/browser

Length of output: 198


🏁 Script executed:

# Check if any actual operation rules set both ReturnType and HasError in practice
rg -A 10 "setTimeout|setInterval|taskHandle" internal/code-gen/customrules/html_rules.go

Repository: gost-dom/browser

Length of output: 966


🏁 Script executed:

# Look at the complete definition of operations that use custom ReturnType
rg "ReturnType:" internal/code-gen/customrules/ -B 2 -A 5

Repository: gost-dom/browser

Length of output: 1120


Clarify method documentation: Custom ReturnType completely overrides default return type construction, including HasError handling.

The behavior is intentional and correct. When o.Rules.ReturnType is set, the method returns immediately with only the custom type, bypassing HasError logic. This is by design—custom types (like TaskHandle for setTimeout/setInterval) are meant to be complete return type definitions that don't require error handling. However, the method comment currently states "If hasError is true, an error return type will be added as well" without explaining that this only applies to the default path, not when a custom ReturnType is specified. Update the comment to explicitly document this precedence to prevent confusion for future maintainers.

Comment on lines 147 to 150
if req.async {
req.DispatchEvent(&event.Event{Type: XHREventLoadstart})
req.clock.AddSafeTask(func() {
req.send(body)
}, 0)
req.clock.EnqueueMacrotask(clock.SafeTask(func() { req.send(body) }))
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Error from send() is silently discarded.

The req.send(body) method returns an error, but wrapping it with clock.SafeTask(func() { req.send(body) }) discards that error. This could lead to silent failures for async XHR requests (e.g., network errors, request creation failures).

Consider handling the error, perhaps by dispatching an error event:

🐛 Proposed fix
 	if req.async {
 		req.DispatchEvent(&event.Event{Type: XHREventLoadstart})
-		req.clock.EnqueueMacrotask(clock.SafeTask(func() { req.send(body) }))
+		req.clock.EnqueueMacrotask(func() error {
+			if err := req.send(body); err != nil {
+				req.logger().Error("XHR send failed", "error", err)
+				req.DispatchEvent(&event.Event{Type: "error"})
+			}
+			return nil
+		})
 		return nil
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if req.async {
req.DispatchEvent(&event.Event{Type: XHREventLoadstart})
req.clock.AddSafeTask(func() {
req.send(body)
}, 0)
req.clock.EnqueueMacrotask(clock.SafeTask(func() { req.send(body) }))
return nil
if req.async {
req.DispatchEvent(&event.Event{Type: XHREventLoadstart})
req.clock.EnqueueMacrotask(func() error {
if err := req.send(body); err != nil {
req.logger().Error("XHR send failed", "error", err)
req.DispatchEvent(&event.Event{Type: "error"})
}
return nil
})
return nil
}

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23d0a40 and d79cc5c.

📒 Files selected for processing (6)
  • html/window_or_worker_global_scope.go
  • internal/clock/clock.go
  • internal/clock/clock_test.go
  • internal/code-gen/interfaces/idl_interface.go
  • internal/code-gen/scripting/codecs.go
  • internal/html/xhr/xml_http_request.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-30T14:56:35.529Z
Learnt from: stroiman
Repo: gost-dom/browser PR: 115
File: scripting/internal/streams/readable_stream_byobreader_generated.go:0-0
Timestamp: 2025-06-30T14:56:35.529Z
Learning: In the gost-dom/browser codebase, files with the `_generated.go` suffix are generated files that often have companion files without that suffix containing additional methods. When reviewing generated files, always check for companion files that may contain missing method implementations before flagging them as issues.

Applied to files:

  • internal/code-gen/scripting/codecs.go
📚 Learning: 2025-05-15T12:03:47.701Z
Learnt from: stroiman
Repo: gost-dom/browser PR: 82
File: scripting/v8host/html_hyperlink_element_utils_generated.go:96-107
Timestamp: 2025-05-15T12:03:47.701Z
Learning: Files with the `_generated` suffix are the result of code generation. Do not provide nitpick reviews for these files, but do report potential actual issues or bugs.

Applied to files:

  • internal/code-gen/scripting/codecs.go
🧬 Code graph analysis (5)
internal/code-gen/interfaces/idl_interface.go (1)
internal/code-gen/customrules/go_type.go (1)
  • GoTypeGenerator (11-11)
html/window_or_worker_global_scope.go (2)
internal/clock/clock.go (2)
  • TaskCallback (24-24)
  • TaskHandle (21-21)
internal/code-gen/gotypes/gotypes.go (1)
  • TaskHandle (24-27)
internal/clock/clock.go (2)
internal/code-gen/packagenames/packagenames.go (1)
  • Clock (17-17)
html/window.go (1)
  • Clock (61-85)
internal/clock/clock_test.go (1)
internal/clock/clock.go (3)
  • TaskCallback (24-24)
  • New (112-123)
  • OfIsoString (442-448)
internal/code-gen/scripting/codecs.go (1)
internal/code-gen/gotypes/gotypes.go (1)
  • TaskHandle (24-27)
⏰ 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). (4)
  • GitHub Check: Tests on linux arm64 with Go
  • GitHub Check: Tests on darwin amd64 with Go
  • GitHub Check: Tests on darwin arm64 with Go
  • GitHub Check: Tests on linux amd64 with Go
🔇 Additional comments (11)
internal/html/xhr/xml_http_request.go (1)

147-153: LGTM! Async send correctly migrated to QueueMacrotask.

The closure properly returns the error from req.send(body), aligning with the new TaskCallback signature (func() error). Discarding the TaskHandle return value is acceptable here since XHR abort functionality is handled separately via Abort().

internal/clock/clock_test.go (3)

101-112: LGTM! Error propagation test correctly validates TaskCallback behavior.

Line 103 properly uses an inline TaskCallback that returns an error, while lines 104-105 use the wrapped version for non-error callbacks. This correctly tests that errors are collected and returned while subsequent tasks still execute.


139-145: LGTM! Tick test validates microtask/macrotask ordering.

The test correctly verifies that AddMicrotask (line 142) executes before QueueMacrotask (line 141), demonstrating proper event loop semantics.


198-216: LGTM! ProcessEvents tests updated to error-returning callbacks.

The AddEvent signature change to accept TaskCallback (returning error) is correctly reflected in the test. The callbacks now explicitly return nil for success cases.

internal/clock/clock.go (3)

26-28: LGTM! SafeTask helper cleanly wraps non-error callbacks.

This provides a convenient way to create a TaskCallback from a simple func() without error handling boilerplate.


376-387: LGTM! SetInterval signature updated to TaskCallback.

The direct assignment of task (line 383) is cleaner than the previous .toTask() conversion pattern. This aligns with the unified TaskCallback API.


403-406: LGTM! QueueMacrotask provides clear intent for zero-delay scheduling.

This method correctly delegates to SetTimeout with a 0 delay, providing semantic clarity for callers who want to queue work for the next event loop iteration rather than scheduling a timeout.

html/window_or_worker_global_scope.go (3)

13-22: LGTM! SetTimeout/ClearTimeout simplified with direct delegation.

The removal of wrapper functions results in cleaner code. The TaskCallback and TaskHandle types provide appropriate type safety for the timer API.


24-33: LGTM! SetInterval/ClearInterval consistently updated.

The same pattern as SetTimeout/ClearTimeout is applied, maintaining API consistency.


35-37: LGTM! QueueMicrotask updated to TaskCallback.

Direct delegation to clock.AddMicrotask is clean and consistent with the new API.

internal/code-gen/interfaces/idl_interface.go (1)

227-242: LGTM!

The branching logic correctly distinguishes between using a custom ReturnType from rules versus the default IDL-derived type. The nullable/bool handling appropriately only applies to the default path, and the GoTypeGenerator usage aligns with the type system introduced in this PR.

Comment on lines +49 to +51
if goType == gotypes.TaskHandle {
return []g.Generator{g.NewValue("decodeTaskHandle")}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for decodeTaskHandle definition in the codebase
echo "=== Searching for decodeTaskHandle function definition ==="
rg -n --type=go 'func\s+[a-zA-Z]*decodeTaskHandle|func\s+[a-zA-Z]*DecodeTaskHandle'

echo ""
echo "=== Searching for all decoder functions in codecs files ==="
fd -e go codecs | xargs rg -n 'func.*Decode' 2>/dev/null | head -30

echo ""
echo "=== Checking the context around lines 47-51 in codecs.go ==="
cat -n internal/code-gen/scripting/codecs.go | sed -n '40,60p'

Repository: gost-dom/browser

Length of output: 1509


🏁 Script executed:

#!/bin/bash
echo "=== Find decodeDuration definition ==="
rg -n 'decodeDuration' internal/code-gen/scripting/codecs.go

echo ""
echo "=== Full content of codecs.go (small file) ==="
wc -l internal/code-gen/scripting/codecs.go

echo ""
echo "=== Check all generators returned in DecodersForGoType ==="
cat -n internal/code-gen/scripting/codecs.go

Repository: gost-dom/browser

Length of output: 3467


Use consistent decoder pattern for decodeTaskHandle.

All other decoders (lines 15-19) are defined as package variables using g.NewValuePackage("DecodeName", packagenames.Codec) and referenced directly. However, decodeTaskHandle on line 50 uses g.NewValue("decodeTaskHandle") with an inline string and has no corresponding variable definition in this file.

Since decodeTaskHandle is defined in scripting/internal/html/window_or_worker_global_scope.go (not in the Codec package), you should either:

  1. Add a variable definition at the top of this file:

    var decodeTaskHandle = g.NewValue("decodeTaskHandle")

    And then use it directly: return []g.Generator{decodeTaskHandle}

  2. Or move DecodeTaskHandle to the Codec package and use the standard pattern like other decoders.

@github-actions github-actions bot merged commit d79cc5c into main Jan 13, 2026
5 checks passed
@github-actions github-actions bot deleted the refactor/decouple-clock branch January 13, 2026 20:39
@coderabbitai coderabbitai bot mentioned this pull request Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants