Skip to content

Comments

fix(fetch): use current global fetch instead of cached one when env f…#2

Open
MitchLewis930 wants to merge 1 commit intopr_022_beforefrom
pr_022_after
Open

fix(fetch): use current global fetch instead of cached one when env f…#2
MitchLewis930 wants to merge 1 commit intopr_022_beforefrom
pr_022_after

Conversation

@MitchLewis930
Copy link

PR_022

@MitchLewis930
Copy link
Author

@greptileai

@greptile-apps
Copy link

greptile-apps bot commented Jan 29, 2026

Greptile Overview

Greptile Summary

This PR resolves a critical issue with Mock Service Worker (MSW) compatibility by changing how the fetch adapter accesses the global fetch function. Previously, the global fetch was cached at module load time in globalFetchAPI, preventing MSW from intercepting requests since MSW replaces the global fetch at runtime.

Key changes:

  • Removed fetch from globalFetchAPI so it's no longer cached at module load
  • Modified factory() to use utils.merge with skipUndefined: true to skip undefined env.fetch values
  • Added runtime resolution: let _fetch = envFetch || fetch allows accessing the current global fetch when env.fetch is not provided
  • Updated isFetchSupported check to evaluate global fetch at runtime when envFetch is undefined
  • Simplified getFetch() by removing redundant merge operation since the merge now happens in factory()
  • Added comprehensive test case that verifies the adapter uses the current global fetch when env.fetch is undefined

The implementation correctly uses the skipUndefined option with utils.merge to ensure that env.fetch = undefined doesn't override the fallback behavior, allowing the adapter to access the global fetch at request time rather than module load time.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is well-designed with proper test coverage. The change correctly uses utils.merge with skipUndefined to handle the undefined env.fetch case, and the runtime fallback logic (_fetch = envFetch || fetch) is sound. The new test specifically validates the intended behavior of using the current global fetch when env.fetch is undefined.
  • No files require special attention

Important Files Changed

Filename Overview
lib/adapters/fetch.js Removes cached global fetch to enable runtime fetch resolution for MSW compatibility
test/unit/adapters/fetch.js Adds test verifying that global fetch is resolved at runtime when env.fetch is undefined

Sequence Diagram

sequenceDiagram
    participant Client
    participant getFetch
    participant factory
    participant Global as Global Scope
    participant Adapter as Fetch Adapter

    Client->>getFetch: config with env.fetch = undefined
    getFetch->>factory: env object
    factory->>factory: merge globalFetchAPI + env (skipUndefined: true)
    Note over factory: env.fetch = undefined is skipped
    factory->>factory: envFetch = undefined
    factory->>Global: Check typeof fetch === 'function'
    Global-->>factory: true/false
    
    Client->>Adapter: Make request
    Adapter->>Adapter: _fetch = envFetch || fetch
    Note over Adapter: Since envFetch is undefined,<br/>uses current global fetch
    Adapter->>Global: Access global fetch at runtime
    Global-->>Adapter: Current fetch implementation<br/>(could be MSW mock)
    Adapter->>Global: _fetch(request)
    Global-->>Adapter: response
    Adapter-->>Client: axios response
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

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