reflex icon indicating copy to clipboard operation
reflex copied to clipboard

Improve handling `Var`'s when passing them as `data` to `rx.download(...)`

Open riebecj opened this issue 2 months ago β€’ 8 comments

Related to #5954 and #5957.

When data is provided as a Var, strings could sometimes have formatting issues and bytes were ArrayVar's that were really not handled at all, as there is no such thing as byte-strings in JavaScript. They were correctly converted to byte arrays, but the resultant downloaded file was just a string representation of the byte array.

  • Var[bytes]: The byte array is coerced into a Uint8Array and converted to a Blob using the specified mime_type. The object URL is then created from the Blob.
  • Var[str]: The string data is left intact to preserve formatting.
  • Var[Any]: Any other supplied state or non-state variable is stringify-ed.
  • Base64 Encoded: To ensure correct data translation, the var data (except for bytes) is base64 encoded.

All of this is done within the variable ecosystem, as there seems to be some aversion to changing any of the Reflex embedded JS (not a gripe, just an observation).

All Submissions:

  • [x] Have you followed the guidelines stated in CONTRIBUTING.md file?
  • [x] Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Please delete options that are not relevant.

  • [x] Bug fix (non-breaking change which fixes an issue)

New Feature Submission:

  • [x] Does your submission pass the tests?
  • [x] Have you linted your code locally prior to submission?

Changes To Core Features:

  • [x] Have you added an explanation of what your changes do and why you'd like us to include them?
  • [ ] Have you written new tests for your core changes, as applicable?
  • [x] Have you successfully ran tests with your changes locally?

riebecj avatar Nov 07 '25 17:11 riebecj

CodSpeed Performance Report

Merging #5961 will not alter performance

Comparing riebecj:main (ea9f14b) with main (3c3ddc2)

Summary

βœ… 8 untouched

codspeed-hq[bot] avatar Nov 07 '25 17:11 codspeed-hq[bot]

I don't think the failed test has anything to do with my code changes, but I can update if necessary.

riebecj avatar Nov 07 '25 17:11 riebecj

@masenf, I can work on that.

What about handling of Python bytes? Currently, nothing in the Var system is designed to ensure handling of Python byte strings to JS byte arrays. Since bytes are a Python primitive, I would suggest that it gets first class treatment before Blobs, urls, and base64 encoding. If the idea is to NOT handle bytes and require conversion to str, it should not be an accepted type anywhere.

riebecj avatar Nov 07 '25 18:11 riebecj

Converting to draft to use as a work space for BytesVar and BlobVar.

This will be abandoned and a new one created when the architecture and implementation of the referenced vars is agreed upon.

riebecj avatar Nov 10 '25 14:11 riebecj

@masenf , Take a look at BytesVar and BlobVar. Let me know where things should be changed/improved/etc.

I ran it through multiple example tests using direct State vars, computed vars, and even creating them directly. All seem to function correctly, download with proper formatting, and supports Unicode.

riebecj avatar Nov 10 '25 14:11 riebecj

All the type hinting seems to be there too.

I was a little wary about using encode and decode because of the underlying _decode for Vars, but it felt the most Pythonic given its implementation type.

riebecj avatar Nov 10 '25 14:11 riebecj

@masenf Made the changes you suggested.

riebecj avatar Nov 11 '25 12:11 riebecj

@masenf Any more suggestions?

riebecj avatar Nov 12 '25 15:11 riebecj