Skip to content

handle upload id being a var in as event spec#5850

Merged
adhami3310 merged 1 commit intomainfrom
handle-upload-id-being-a-var
Oct 3, 2025
Merged

handle upload id being a var in as event spec#5850
adhami3310 merged 1 commit intomainfrom
handle-upload-id-being-a-var

Conversation

@adhami3310
Copy link
Member

No description provided.

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

Summary

This PR fixes a bug in the `FileUpload.as_event_spec` method where the `upload_id` assignment logic was incorrectly using the logical OR operator (`or`), which would treat any falsy value as a reason to fall back to the default upload ID. The change replaces this with an explicit None check (`is not None`), ensuring that only `None` values trigger the default behavior.

The fix is crucial for the Reflex framework's file upload functionality because upload_id can be a Var object representing dynamic values from the frontend. These Var objects might evaluate to falsy values (like empty strings, 0, False, etc.) while still being valid and intentional upload IDs. The original logic would incorrectly replace these valid falsy values with DEFAULT_UPLOAD_ID, breaking the intended functionality when users provide specific upload IDs that happen to be falsy.

This change integrates well with Reflex's reactive state system, where Var objects are commonly used to represent dynamic frontend values that can change during runtime. By preserving all non-None values, the fix maintains the integrity of user-specified upload IDs regardless of their truthiness.

Changed Files
Filename Score Overview
reflex/event.py 5/5 Fixed upload_id assignment logic to preserve falsy but valid values using explicit None check

Confidence score: 5/5

  • This PR is safe to merge with minimal risk
  • Score reflects a simple, well-targeted bug fix that addresses a clear logical error without introducing complexity
  • No files require special attention

Sequence Diagram

sequenceDiagram
    participant User
    participant FileUploadComponent
    participant FileUpload
    participant EventSpec
    participant LiteralVar
    participant VarData

    User->>FileUploadComponent: "Initiates file upload with dynamic upload_id"
    FileUploadComponent->>FileUpload: "Creates FileUpload instance with upload_id as Var"
    FileUpload->>FileUpload: "as_event_spec(handler) called"
    
    alt upload_id is None
        FileUpload->>FileUpload: "Use DEFAULT_UPLOAD_ID"
    else upload_id is not None
        FileUpload->>FileUpload: "Use provided upload_id (string or Var)"
    end
    
    FileUpload->>LiteralVar: "create(upload_id) for filesById access"
    LiteralVar-->>FileUpload: "Returns LiteralVar wrapping upload_id"
    
    FileUpload->>VarData: "merge(upload_files_context_var_data)"
    VarData-->>FileUpload: "Returns merged var data"
    
    FileUpload->>EventSpec: "Create spec_args with files, upload_id, extra_headers"
    
    opt on_upload_progress is provided
        FileUpload->>EventChain: "Create progress event chain"
        EventChain-->>FileUpload: "Returns formatted chain"
        FileUpload->>EventSpec: "Add on_upload_progress to spec_args"
    end
    
    FileUpload->>EventSpec: "Create EventSpec with uploadFiles client handler"
    EventSpec-->>FileUploadComponent: "Returns complete event specification"
    FileUploadComponent-->>User: "File upload ready with dynamic upload_id support"
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 3, 2025

CodSpeed Performance Report

Merging #5850 will not alter performance

Comparing handle-upload-id-being-a-var (0b5c557) with main (5f3bf56)

Summary

✅ 8 untouched

@adhami3310 adhami3310 merged commit 09c33a8 into main Oct 3, 2025
39 of 41 checks passed
@adhami3310 adhami3310 deleted the handle-upload-id-being-a-var branch October 3, 2025 18:25
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