Skip to content

Conversation

@adhami3310
Copy link
Member

No description provided.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 6, 2025

CodSpeed Performance Report

Merging #5955 will not alter performance

Comparing add-polling-transport-option (8bc94b3) with main (579e922)

Summary

✅ 8 untouched

Copy link
Contributor

@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.

Greptile Overview

Greptile Summary

This PR adds configurable polling transport support as an alternative to WebSocket connections. The implementation allows users to set transport: "polling" or transport: "websocket" in their config, and this value flows through to both the backend Socket.IO server and frontend client.

Key Changes:

  • Added transport config field with Literal typing ("websocket" | "polling")
  • Modified Socket.IO server initialization to use config.transport instead of hardcoded ["websocket"]
  • Updated frontend to read transport from env.TRANSPORT instead of hardcoded value
  • Added TRANSPORT key to env.json generation

Critical Issues:

  • CORS Configuration Regression: The PR hardcodes cors_allowed_origins=[] and cors_credentials=False, completely ignoring user's config.cors_allowed_origins setting. This breaks existing apps with custom CORS requirements. Socket.IO polling requires CORS configuration for the initial HTTP handshake.
  • Transport Upgrade Blocked: allow_upgrades=False prevents automatic upgrade from polling to WebSocket, which may not be the desired behavior for all use cases.

Confidence Score: 1/5

  • This PR has critical regressions that will break existing production apps with custom CORS configurations
  • The hardcoded CORS configuration (cors_allowed_origins=[], cors_credentials=False) removes support for user-configured CORS settings that were previously respected. This is a breaking change that will prevent WebSocket/polling connections in production environments where CORS is properly configured. The feature implementation itself is sound, but the CORS regression must be fixed before merging.
  • Pay critical attention to reflex/app.py - the CORS configuration must be restored to respect config.cors_allowed_origins

Important Files Changed

File Analysis

Filename Score Overview
reflex/app.py 1/5 Critical CORS configuration regression - hardcoded empty array breaks user config; allow_upgrades=False may prevent websocket upgrade
reflex/config.py 5/5 Added transport config option with proper Literal typing - clean implementation
reflex/utils/build.py 4/5 Added TRANSPORT to env.json - minor style issue with hardcoded string literal
reflex/.templates/web/utils/state.js 5/5 Changed from hardcoded "websocket" to dynamic env.TRANSPORT - correct implementation

Sequence Diagram

sequenceDiagram
    participant User as User Config
    participant Build as build.py
    participant EnvJSON as env.json
    participant Frontend as state.js
    participant App as app.py
    participant SocketIO as Socket.IO Server

    User->>App: Set transport config
    Note over User,App: Config: transport = "websocket" | "polling"
    
    Build->>User: Read config.transport
    Build->>EnvJSON: Write TRANSPORT value
    Note over EnvJSON: env.json contains TRANSPORT key
    
    Frontend->>EnvJSON: Read env.TRANSPORT
    Frontend->>SocketIO: Connect with [env.TRANSPORT]
    Note over Frontend,SocketIO: Previously hardcoded ["websocket"]
    
    App->>SocketIO: Initialize AsyncServer
    Note over App,SocketIO: transports=[config.transport]<br/>allow_upgrades=False<br/>CORS hardcoded to []
    
    alt Websocket Transport
        SocketIO-->>Frontend: WebSocket connection
    else Polling Transport
        SocketIO-->>Frontend: HTTP long-polling
        Note over SocketIO,Frontend: Cannot upgrade to WebSocket
    end
Loading

4 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@adhami3310 adhami3310 merged commit fd6e111 into main Nov 6, 2025
47 checks passed
@adhami3310 adhami3310 deleted the add-polling-transport-option branch November 6, 2025 22:10
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.

3 participants