Skip to content

Add native fetch() polyfill - #188

Merged
bkaradzic-microsoft merged 6 commits into
BabylonJS:mainfrom
bkaradzic-microsoft:weekend/tpc-1581-fetch-native
Jun 10, 2026
Merged

Add native fetch() polyfill#188
bkaradzic-microsoft merged 6 commits into
BabylonJS:mainfrom
bkaradzic-microsoft:weekend/tpc-1581-fetch-native

Conversation

@bkaradzic-microsoft

Copy link
Copy Markdown
Member

Implements the WHATWG etch() API as a native polyfill under Polyfills/Fetch/, mirroring the XMLHttpRequest polyfill layout. Like XMLHttpRequest, it is built on the same platform-specific UrlLib transports, so libcurl/WinHTTP/etc. behavior is identical between the two.

Closes#98.

API

fetch(input, init) returns a Promise that resolves to a Response-like object exposing:

  • ok, status, statusText, url, redirected, type, bodyUsed
  • headers with get(name) / has(name) / forEach(cb) (case-insensitive)
  • text(), arrayBuffer(), json(), blob() (each returns a Promise)
  • clone()

Behavior / deliberate limitations

  • Per the fetch spec, the promise rejects only on transport-level failures; a completed request with a non-2xx status (e.g. 404) still resolves with ok === false.
  • The body is fully buffered before the promise resolves, so the body accessors may be called more than once (bodyUsed stays false) — a lenient deviation from the spec's single-use semantics.
  • GET/POST only and string request bodies only, matching the underlying UrlLib transport (same constraints as XMLHttpRequest).
  • blob() requires the Blob polyfill to be initialized.

Wiring

  • New JSRUNTIMEHOST_POLYFILL_FETCH option (default ON). UrlLib is now fetched when eitherXMLHttpRequest or Fetch is enabled.
  • Initialized in the test harness and covered by a new describe("fetch") block (12 cases: status/ok, 404-resolves, statusText, text/arrayBuffer/json/blob, case-insensitive headers, clone, init method, no-arg rejection). A small Assets/sample.json backs the deterministic json() test.

Verification

Built the Fetch target under warnings_as_errors and ran the full UnitTests suite on Win32/Chakra: 192 passing, including all 12 new fetch cases.

Context

This is step 2 of the fetch native-polyfill pivot discussed in BabylonNative#1707 (reviewers asked fetch to follow the same native pattern as File/FileReader (#169) and AbortController). Once this lands, BabylonNative#1707 becomes a small bump that initializes Polyfills::Fetch.

CopilotAI review requested due to automatic review settings June 6, 2026 00:02

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a native fetch() polyfill implemented in C++ on top of the existing UrlLib transports (matching the XMLHttpRequest approach), wires it behind a new CMake option, and extends the unit test suite to validate core fetch/Response behaviors.

Changes:

  • Introduce Polyfills/Fetch library implementing fetch(input, init) and a minimal Response/Headers surface.
  • Wire JSRUNTIMEHOST_POLYFILL_FETCH (default ON) and ensure UrlLib is fetched when either XHR or Fetch is enabled.
  • Add Mocha unit tests and a JSON asset to validate response properties and body readers (text/arrayBuffer/json/blob) plus cloning and header behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
Tests/UnitTests/Shared/Shared.cppInitializes the new Fetch polyfill in the UnitTests runtime.
Tests/UnitTests/Scripts/tests.tsAdds a new describe("fetch") suite covering status/ok, statusText, body readers, headers, clone, init.method, and no-arg rejection.
Tests/UnitTests/CMakeLists.txtLinks the new Fetch target into the UnitTests executable; Assets are globbed/copied so sample.json is picked up.
Tests/UnitTests/Assets/sample.jsonAdds deterministic JSON payload for the response.json() test.
Polyfills/Fetch/Source/Fetch.hDeclares internal Fetch initializer.
Polyfills/Fetch/Source/Fetch.cppImplements fetch(), Response-like object, and headers helpers over UrlLib.
Polyfills/Fetch/Readme.mdDocuments supported surface area and known limitations.
Polyfills/Fetch/Include/Babylon/Polyfills/Fetch.hAdds public API header for initializing the polyfill.
Polyfills/Fetch/CMakeLists.txtDefines the Fetch library target and links against JsRuntime, arcana, and UrlLib.
Polyfills/CMakeLists.txtAdds conditional inclusion of the Fetch subdirectory.
CMakeLists.txtAdds JSRUNTIMEHOST_POLYFILL_FETCH option and updates UrlLib dependency gating.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadPolyfills/Fetch/Source/Fetch.cpp
Comment threadPolyfills/Fetch/Source/Fetch.cpp
@bkaradzic-microsoft

Copy link
Copy Markdown
MemberAuthor

Addressed both review comments in ebe6a80:

  • blob() on JSC/JSI: switched the Blob-availability check from IsFunction() to IsUndefined()/IsNull(), matching the existing File polyfill workaround. Some JavaScriptCore/JSI builds report constructor functions as typeof 'object', which was causing the \�lob()\ test to fail across all JSC/JSI CI jobs.
  • : added the explicit include for \std::tolower/\std::toupper.

This should turn the failing JSC/JSI/Android/Ubuntu/macOS/iOS builds green (they were all failing solely on the blob() test).

@bkaradzic-microsoft

Copy link
Copy Markdown
MemberAuthor

CI is now fully green (23/23 ✅). Pushed four follow-up fixes after the initial submission, all surfaced by platform-specific CI:

  1. *\40f5ff7* — Fixed a stack-use-after-return crash (\std::system_error: Invalid argument\ on clang/libc++, JSC, JSI, Android). arcana's \ ask::then()\ captures the scheduler by reference and runs it on the UrlLib worker thread after \ etch()\ has returned, so the stack-local \JsRuntimeScheduler\ dangled. Now heap-allocated and co-owned by the continuation (mirrors how XMLHttpRequest keeps its scheduler alive as a member). Caught by ASAN.
  2. *\�be6a80* — \�lob()\ now detects the Blob polyfill via \IsUndefined()/IsNull()\ instead of \IsFunction()\ (JSC/JSI report constructors as typeof 'object'), plus added <cctype>. (addresses both Copilot review comments)
  3. *\44bc77a* — Made the \�lob()\ \parts/\options\ Napi objects non-const (Node-API-JSI declares \Set()\ non-const → C2663).
  4. *\1118799* — Linked \Fetch\ into the Android \UnitTestsJNI\ target, which has its own polyfill link list.

Ready for re-review.

bkaradzicand others added 5 commits June 9, 2026 09:29
Implements the WHATWG fetch() API as a native polyfill under
Polyfills/Fetch/, mirroring the XMLHttpRequest polyfill layout. Like
XMLHttpRequest, it is built on top of the platform-specific transports
in UrlLib, so libcurl/WinHTTP/etc. behavior is identical between the two.
fetch(input, init) returns a Promise that resolves to a Response-like
object exposing ok/status/statusText/url/redirected/type/bodyUsed, a
case-insensitive headers accessor (get/has/forEach), the body accessors
text()/arrayBuffer()/json()/blob(), and clone(). Per the fetch spec, the
promise only rejects on transport-level failures; a completed request
with a non-2xx status (e.g. 404) still resolves with ok === false.
The body is fully buffered before the promise resolves, so the body
accessors may be called more than once (a deliberate lenient deviation
from the spec's single-use semantics). Methods are limited to GET/POST
and request bodies to strings, matching the underlying UrlLib transport.
Wired behind the JSRUNTIMEHOST_POLYFILL_FETCH option (UrlLib is now
fetched when either XMLHttpRequest or Fetch is enabled) and covered by
new unit tests.
ClosesBabylonJS#98.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- blob() now detects the Blob polyfill via IsUndefined()/IsNull() instead
of IsFunction(). Some JavaScriptCore/JSI builds classify constructor
functions as typeof 'object', so IsFunction() incorrectly rejected even
when the Blob polyfill was installed, failing the blob() test across all
JSC/JSI CI jobs (matches the existing File polyfill workaround).
- Add <cctype> for std::tolower/std::toupper used by ToLower/StatusText,
which were relying on transitive includes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
arcana's task::then() captures the scheduler by reference and invokes it
on the UrlLib worker thread when the request completes, which happens after
the synchronous fetch() call has already returned. The previous stack-local
JsRuntimeScheduler was therefore destroyed before the continuation ran,
leaving arcana with a dangling reference. On Windows/Chakra this happened to
survive, but on clang/libc++, JSC, JSI and Android it dereferenced freed
memory and aborted with 'std::system_error: Invalid argument' inside
JsRuntime::Dispatch (caught by ASAN as a stack-use-after-return).
Heap-allocate the scheduler in a shared_ptr and co-own it from the
continuation callable so it stays alive until the request completes,
mirroring how XMLHttpRequest keeps its scheduler alive as a member.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Node-API-JSI declares Napi::Object::Set / Napi::Array::Set as non-const
member functions, so calling Set() on 'const auto' instances failed to
compile (C2663) on the *_JSI and Android targets while compiling fine
under Chakra/V8. Declare the blob() 'parts' array and 'options' object
as non-const, matching the other builder objects in this file.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Android UnitTests app uses its own CMakeLists with an explicit polyfill
link list, separate from the desktop Tests/UnitTests target. Without Fetch
there, Shared.cpp failed to find <Babylon/Polyfills/Fetch.h> on Android_JSC
and Android_V8. Add 'PRIVATE Fetch' to mirror the desktop test target.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bkaradzic-microsoft
bkaradzic-microsoftforce-pushed the weekend/tpc-1581-fetch-native branch from 1118799 to fbe94dbCompareJune 9, 2026 16:29

@bghgarybghgary left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[Reviewed by Copilot on behalf of @bghgary]

Clean polyfill. One correctness fix and one optional cleanup inline.

Comment threadPolyfills/Fetch/Source/Fetch.cpp Outdated
Comment threadPolyfills/Fetch/Source/Fetch.cpp Outdated
Comment threadPolyfills/Fetch/Source/Fetch.cpp Outdated
Comment threadPolyfills/Fetch/Source/Fetch.cpp
Comment threadPolyfills/Fetch/Source/Fetch.cpp Outdated
Comment threadPolyfills/Fetch/Source/Fetch.cpp Outdated
Comment threadPolyfills/Fetch/Source/Fetch.cpp Outdated
Comment threadPolyfills/Fetch/Source/Fetch.cpp Outdated
Comment threadPolyfills/Fetch/Source/Fetch.cpp
…thod compares
Address review feedback on the native fetch() polyfill:
- Fix a hang: a throw inside the SendAsync continuation (e.g. from
BuildResponse) was captured into the discarded task result and dropped,
so the promise never settled and await fetch(...) hung. Split into two
chained .then() continuations -- the first does the work and resolves,
the second surfaces any error as a promise rejection. The scheduler is
co-owned by the second continuation so it outlives the request.
- Replace ToLower with a non-allocating EqualsIgnoreCase comparator used
by both ParseMethod and FindHeader (removes per-header allocations).
- Make ResponseData::body a plain std::vector<std::byte> instead of a
shared_ptr; the struct is always held by shared_ptr, so the inner
pointer only added an allocation and falsely implied clones get
independent bodies.
- Remove the unnecessary BuildResponse forward declaration.
- Collapse the two fetch() catch blocks into a single catch (...) via the
exception_ptr Error overload, removing the message-prefix asymmetry.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkaradzic-microsoft pushed a commit to bkaradzic-microsoft/JsRuntimeHost that referenced this pull request Jun 10, 2026
Follow-up to BabylonJS#188. UrlLib now exposes UrlRequest::StatusText() (the wire
reason phrase where available, else a canonical code->text table), so:
- Fetch: drop the private status-code->text table and read
request.StatusText() into Response.statusText.
- XMLHttpRequest: expose statusText (previously absent) for free.
Adds fetch + XMLHttpRequest statusText tests (OK / Not Found).
Bumps the UrlLib pin to BabylonJS/UrlLib 74985214, which adds
UrlRequest::StatusText() (BabylonJS/UrlLib#30).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bkaradzic-microsoft

Copy link
Copy Markdown
MemberAuthor

Thanks for the review, @bghgary — all comments are addressed in ac2019f, and the follow-ups you flagged are now resolved/tracked.

Correctness fix (the blocker): the SendAsync() continuation is split into two chained .then()s — the first does the work and resolves, the second inspects the expected result and rejects on error. So a throw inside the continuation (e.g. from BuildResponse) now settles the promise as a rejection instead of being captured into a discarded result and dropped, which previously hung await fetch(...). The scheduler is co-owned by the second continuation so it outlives the in-flight request.

Cleanups: single catch (...) via the exception_ptrNapi::Error overload (removes the Napi::Error-vs-std::exception prefix asymmetry); non-allocating EqualsIgnoreCase(string_view, string_view) replacing ToLower, used by both ParseMethod and FindHeader; ResponseData::body is now a plain std::vector<std::byte>; removed the unnecessary BuildResponse forward declaration.

statusText "belongs in UrlLib" — done.BabylonJS/UrlLib#30 is merged and exposes UrlRequest::StatusText() (wire reason phrase where available, else a canonical code→text fallback table). The stacked follow-up #195 drops the private table here and reads request.StatusText() into Response.statusText (and gives XMLHttpRequest.statusText for free). Tracked + closed via #193.

Node-API-JSC napi_typeof bug (the blob()IsFunction() workaround): root cause is napi_typeof deciding function-ness solely via JSObjectIsFunction, which returns false for JSObjectMakeConstructor constructors on some JSC builds, so it reports napi_object where V8 returns napi_function. Tracked in #194.

CI is green. Re-requested your review — PTAL.

@bghgarybghgary left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[Reviewed by Copilot on behalf of @bghgary]

LGTM

@bkaradzic-microsoft
bkaradzic-microsoft merged commit a407a33 into BabylonJS:mainJun 10, 2026
23 checks passed
bkaradzic-microsoft pushed a commit to bkaradzic-microsoft/JsRuntimeHost that referenced this pull request Jun 10, 2026
Follow-up to BabylonJS#188. UrlLib now exposes UrlRequest::StatusText() (the wire
reason phrase where available, else a canonical code->text table), so:
- Fetch: drop the private status-code->text table and read
request.StatusText() into Response.statusText.
- XMLHttpRequest: expose statusText (previously absent) for free.
Adds fetch + XMLHttpRequest statusText tests (OK / Not Found).
Bumps the UrlLib pin to BabylonJS/UrlLib 74985214, which adds
UrlRequest::StatusText() (BabylonJS/UrlLib#30).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkaradzic-microsoft pushed a commit to bkaradzic-microsoft/JsRuntimeHost that referenced this pull request Jun 10, 2026
Follow-up to BabylonJS#188. UrlLib now exposes UrlRequest::StatusText() (the wire
reason phrase where available, else a canonical code->text table), so:
- Fetch: drop the private status-code->text table and read
request.StatusText() into Response.statusText.
- XMLHttpRequest: expose statusText (previously absent) for free.
Adds fetch + XMLHttpRequest statusText tests (OK / Not Found).
Bumps the UrlLib pin to BabylonJS/UrlLib 74985214, which adds
UrlRequest::StatusText() (BabylonJS/UrlLib#30).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkaradzic-microsoft added a commit to BabylonJS/BabylonNative that referenced this pull request Jun 12, 2026
Wires Babylon Native's Playground up to the **native `fetch()`
polyfill** that now lives in JsRuntimeHost
(BabylonJS/JsRuntimeHost#188), so playgrounds depending on `fetch` stop
hitting `ReferenceError: 'fetch' is not defined`.
This supersedes the original JS XMLHttpRequest-based shim approach:
rather than shipping a `fetch_polyfill.js` script, `fetch` is now
provided natively (built on the same `UrlLib` transport as
`XMLHttpRequest`), matching how File/FileReader landed in #1706.
**Changes:**
- `Apps/Playground/Shared/AppContext.cpp` - calls
`Babylon::Polyfills::Fetch::Initialize(env)` alongside the other
polyfills (right after `XMLHttpRequest`).
- `Apps/Playground/CMakeLists.txt` +
`Apps/Playground/Android/BabylonNative/CMakeLists.txt` - link the
`Fetch` target.
- `CMakeLists.txt` - bumps the JsRuntimeHost pin to pick up the native
polyfill.
> ✅ **Unblocked:** JsRuntimeHost#188 merged, and the pin now points at
`BabylonJS/JsRuntimeHost@a407a331` (the #188 merge commit) — no longer
the temporary fork branch. Rebased onto master (clean).
**Verified (Win32/Chakra):** `Playground` configures, builds and links
cleanly against the native polyfill, and at runtime `typeof fetch ===
"function"` (confirmed via a headless smoke check).
---
## Landing context
This PR is one of the **splits** from the proven CI-green combined
preview in draft PR #1702 (see
[#1702](#1702) for the
full intended end-state).
> The original split also included #1709 (ES2020+ -> ES2019
syntax-repair polyfill for Chakra), closed in favour of investigating
`@babel/standalone` properly (#1711).
### Landing order / status
**Tier 1 - parallel-reviewable, no source conflicts:**
1. ~~#1703 - ExternalTexture C4702 build fix~~ ✅ merged
2. ~~#1704 - config.json `reason` rewrites (5 entries)~~ ✅ merged
3. ~~#1705 - config.json `reason` rewrites (17 entries)~~ ✅ merged
**Tier 2 - sequential, each touches `Apps/Playground/CMakeLists.txt` +
`AppContext.cpp`:**
4. ~~#1706 - File/FileReader polyfill~~ ✅ merged (native JsRuntimeHost
bump)
5. **#1707 - native fetch polyfill (this PR)** - ✅ unblocked; pin
repointed to `BabylonJS/JsRuntimeHost@a407a331`; ready for review
6. #1708 - link AbortController + TextEncoder polyfills from
JsRuntimeHost
7. #1710 - Cubemap auto-expand polyfill (loaded after babylon.max.js)
### Reference policy reminder
Reference PNGs come from Babylon.js; never re-baked by BN. Combined
diff: **0 PNGs**.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bkaradzic-microsoft
bkaradzic-microsoft deleted the weekend/tpc-1581-fetch-native branch July 30, 2026 23:09
Sign up for freeto 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.

Implement fetch API

4 participants

@bkaradzic-microsoft@bghgary@bkaradzic