Skip to content

Comments

Fixing defaults to use httpAdapter if available (#1285)#9

Open
MitchLewis930 wants to merge 1 commit intopr_029_beforefrom
pr_029_after
Open

Fixing defaults to use httpAdapter if available (#1285)#9
MitchLewis930 wants to merge 1 commit intopr_029_beforefrom
pr_029_after

Conversation

@MitchLewis930
Copy link

PR_029

* Fixing defaults to use httpAdapter if available
* Use a safer, cross-platform method to detect the Node environment
@greptile-apps
Copy link

greptile-apps bot commented Jan 29, 2026

Greptile Overview

Greptile Summary

Improved environment detection in getDefaultAdapter() to correctly identify Node.js vs browser environments when webpack polyfills are present.

Key changes:

  • Reordered adapter detection to check Node.js environment first, then browser
  • Added robust Node.js detection using Object.prototype.toString.call(process) === '[object process]' instead of just checking typeof process
  • Prevents incorrect adapter selection when webpack's DefinePlugin creates a partial process object in browser bundles

The more robust check ensures that the HTTP adapter is used in actual Node.js environments, while the XHR adapter is correctly selected in browsers even when webpack polyfills create a process.env object.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change uses a well-established pattern for Node.js detection (same pattern used in es6-promise polyfill). The logic is straightforward, improves environment detection accuracy, and addresses a real issue with webpack polyfills creating partial process objects
  • No files require special attention

Important Files Changed

Filename Overview
lib/defaults.js Improved adapter detection logic to prioritize Node.js HTTP adapter with robust environment check using Object.prototype.toString

Sequence Diagram

sequenceDiagram
    participant App as Application Code
    participant Defaults as lib/defaults.js
    participant HttpAdapter as lib/adapters/http.js
    participant XhrAdapter as lib/adapters/xhr.js
    
    App->>Defaults: Import and initialize defaults
    Defaults->>Defaults: Call getDefaultAdapter()
    
    alt Running in Node.js
        Defaults->>Defaults: Check typeof process !== 'undefined'
        Defaults->>Defaults: Check Object.prototype.toString.call(process) === '[object process]'
        Defaults->>HttpAdapter: require('./adapters/http')
        HttpAdapter-->>Defaults: Return HTTP adapter function
        Defaults-->>App: Use HTTP adapter (Node.js)
    else Running in Browser
        Defaults->>Defaults: Check typeof process !== 'undefined' (may exist due to webpack)
        Defaults->>Defaults: Check Object.prototype.toString.call(process) === '[object process]' (fails for webpack polyfill)
        Defaults->>Defaults: Check typeof XMLHttpRequest !== 'undefined'
        Defaults->>XhrAdapter: require('./adapters/xhr')
        XhrAdapter-->>Defaults: Return XHR adapter function
        Defaults-->>App: Use XHR adapter (Browser)
    end
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.

1 file 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.

1 participant