Flesh out URL polyfill and add performance.now polyfill#130
Merged
ryantrem merged 16 commits intoBabylonJS:mainfrom Dec 12, 2025
Merged
Flesh out URL polyfill and add performance.now polyfill#130ryantrem merged 16 commits intoBabylonJS:mainfrom
ryantrem merged 16 commits intoBabylonJS:mainfrom
Conversation
…versions of iOS In napi_remove_wrap the result param is optional, so check it before trying to assign to it (it is null when a constructor throws an error for example) Remove finalizers when napi_remove_wrap is called (this is what the napi spec says to do, and this corresponds to existing issue BabylonJS#82) Flesh out the URL polyfill and add tests
… ignore any beyond 2 Only check OS version with __builtin_available when compiling on APPLE platforms
…mething specific about how the test framework is getting the type of a value?)
bghgary
approved these changes
Dec 12, 2025
Contributor
bghgary
left a comment
bghgary
left a comment
There was a problem hiding this comment.
I only skimmed the URL parsing code. I think we have to be careful with this code as it can introduce issues that we don't see. I still think we should use platform specific URL code to avoid this issue entirely.
This was referenced Apr 17, 2026
bghgary
added a commit
that referenced
this pull request
Apr 17, 2026
[Created by Copilot on behalf of @bghgary] Re-enable the "should trigger error callback with invalid server" WebSocket test that was commented out in #130 due to the flake reported in #131. ## Why this is safe to re-enable Two WebSocket-related fixes landed on `main` since the flake was observed: - **#150** — pulled in UrlLib BabylonJS/UrlLib#26, which consolidates `onError`/`onClose` dispatch on Windows and Apple. Before this, the Windows and Apple implementations only fired `onError` on connection failures without a matching `onClose`, leaving consumers (and this test) in inconsistent terminal-event states. - **#160** — made the JS-layer `close()` idempotent and `send()` spec-compliant when CLOSING/CLOSED, fixing cross-state dispatch bugs. ## Verification Repro PR #161 expanded this test to 20 sequential iterations on the same branch to exercise the historical 1/3 flake rate. If the flake persisted, P(all 20 pass) ≈ 0.03%. CI on #161 ran across every platform/engine combo — Chakra/V8/JSI on UWP/Win32, Linux (gcc, clang, sanitizers, TSan), macOS (Xcode 16.4, sanitizers, TSan), iOS (Xcode 15.2, 16.4) — and **all 20 iterations passed on every job** ([workflow run 24587856930](https://github.com/BabylonJS/JsRuntimeHost/actions/runs/24587856930)). This PR restores the original single test (no loop, default mocha timeout) as it was before #130. Fixes #131. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
URLpolyfill (moslty by Claude).URL(by Claude).performance.now()(by Claude).performance.now()(by Claude).JSGlobalContextSetInspectableand add platform and os version checks.napi_remove_wrapis called (per https://nodejs.org/api/n-api.html#napi_remove_wrap). Otherwise, a crash occurs if a constructor throws an error (which can happen with URL when the passed in string is not a valid url). This also addresses Node-API napi_remove_wrap is not being handled correctly #82.napi_remove_wrapwhen thevoid** resultis null. We just need to checkresultbefore assigning to it.resultcan be null in certain situations, such as if a constructor throws (same scenario as above).