Skip to content

fix: emit valid JS for DestructuredArg with only a rest field#6444

Open
FarhanAliRaza wants to merge 1 commit intoreflex-dev:mainfrom
FarhanAliRaza:fix-syntax-rxprops
Open

fix: emit valid JS for DestructuredArg with only a rest field#6444
FarhanAliRaza wants to merge 1 commit intoreflex-dev:mainfrom
FarhanAliRaza:fix-syntax-rxprops

Conversation

@FarhanAliRaza
Copy link
Copy Markdown
Contributor

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • 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.

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

New Feature Submission:

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

Changes To Core Features:

  • 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?
  • Have you successfully ran tests with your changes locally?

closes #6443

@FarhanAliRaza FarhanAliRaza requested a review from a team as a code owner May 2, 2026 19:32
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 2, 2026

Greptile Summary

This PR fixes DestructuredArg.to_javascript() to emit valid JavaScript when fields is empty and only a rest element is present — previously producing {, ...rest} instead of {...rest}. Two regression tests are added to cover both component-returning and var-returning memo variants, and an existing test's monkeypatch targets are corrected from compiler.utils to the explicitly imported compiler_utils alias.

Confidence Score: 5/5

Safe to merge — minimal, well-targeted fix with regression tests covering all affected code paths.

The change is a single-method, two-branch fix that handles all four combinations of empty/non-empty fields and present/absent rest. The new tests directly assert both the correct output and the absence of the previously broken form. No other logic is touched.

No files require special attention.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/vars/function.py Fixes DestructuredArg.to_javascript() to omit the leading comma when fields is empty and only a rest element is present, preventing invalid JS like {, ...rest}.
tests/units/experimental/test_memo.py Adds two regression tests for rest-only memo args, and corrects monkeypatch targets from compiler.utils to the already-imported compiler_utils alias for the existing import-remerging test.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["DestructuredArg.to_javascript()"] --> B{"fields non-empty?"}
    B -- Yes --> C["inner = ', '.join(fields)"]
    B -- No --> D["inner = ''"]
    C --> E{"rest set?"}
    D --> E
    E -- Yes, inner non-empty --> F["inner = '{inner}, ...{rest}'"]
    E -- Yes, inner empty --> G["inner = '...{rest}'"]
    E -- No --> H["inner unchanged"]
    F --> I["return wrap(inner, '{', '}')"]
    G --> I
    H --> I
    I --> J["Result: valid JS\ne.g. {a, b, ...rest} or {...rest} or {a, b} or {}"]
Loading

Reviews (1): Last reviewed commit: "fix: emit valid JS for DestructuredArg w..." | Re-trigger Greptile

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 2, 2026

Merging this PR will not alter performance

✅ 17 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing FarhanAliRaza:fix-syntax-rxprops (4044c1c) with main (70ab07e)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

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.

[rx._x.memo] invalid syntax when using only rx.RestProp

1 participant