Skip to content

fix: isolate dynamic JavaScript execution environments - #7870

Merged
FarhanAliRaza merged 12 commits into
reflex-dev:mainfrom
somiljain2006:Eval
Nov 13, 2026
Merged

FarhanAliRaza merged 12 commits into
reflex-dev:mainfrom
somiljain2006:Eval

Conversation

@somiljain2006

@somiljain2006 somiljain2006 commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

All Submissions:

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

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality not to work as expected)
  • This change requires a documentation update

New Feature Submission:

  • Does your submission pass the tests?
  • Have you linted your code locally before 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 run tests with your changes locally?

closes #7860

Server-provided JavaScript (rx.call_script code, rx.call_function functions and their result callbacks) no longer runs through a direct eval in utils/state.js. A direct eval stops the minifier from mangling the chunk that holds it; with frontend_lazy_bundled_libraries on, this shrinks the shared chunk by about 88 KB (11 KB gzipped).

  • One runScript helper compiles a Function-constructor evaluator on first use, so a CSP without unsafe-eval still loads apps that never send scripts. Its body passes the code to eval, which sees only the Function's parameters and the globals, and runs it as a strict script, so statement lists keep their completion value.
  • Scripts see every export of utils/state.js (ReflexEvent, refs, queueEvents, pyMod, ...), the late-bound addEvents, the socket, navigate and params of the event, the session token, and the globals. Names the module does not export (its imports, such as eventLoop) are no longer in scope.
  • evalReactComponent uses a native import() of the data URI instead of eval("import(...)").
  • Tests: packages/reflex-frontend/tests/run_script.test.js pins completion values, strict mode, scope, late-bound addEvents across event-loop remounts, and lazy compilation. test_state_js_template.py checks that every runScript call site declares the addEvents alias first.

View guided diff

@somiljain2006
somiljain2006 requested a review from a team as a code owner November 12, 2026 18:14
@greptile-apps

greptile-apps Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the latest change fixes the release note without changing runtime behavior.

Summary

The PR gives server-provided JavaScript an explicit set of available names, allowing smaller frontend bundles while preserving script results and event callbacks.

  • Server-provided scripts run with an explicit set of names.
  • Dynamic components load through native imports.

somiljain2006 accepted the inline dependency-name duplication because it matches the surrounding style and is small. No new issues found.

Reviews (9) · Last reviewed commit: "Describe the actual script scope in the ..." · Reviewed by Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

@masenf masenf added the on deck PRs lined up to review / merge next label Nov 12, 2026
@codspeed

codspeed Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 2 benchmarks

1 warning

⚠️ 18 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚡ 2 improved benchmarks
❌ 2 regressed benchmarks
✅ 147 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ test_state_update_wire_serialization[model_1000] 25.2 ms 26.6 ms -5.28%
❌ test_process_event_burst_independent_tokens[1] 2.7 ms 2.8 ms -4.26%
⚡ test_state_collection_assignment[1000] 12.1 ms 10.7 ms +12.66%
⚡ test_state_collection_assignment[100000] 1.1 s 1 s +5.27%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing somiljain2006:Eval (425e405) with main (b5aff10)

Open in CodSpeed

somiljain2006 and others added 3 commits November 13, 2026 03:07
Generated callbacks call any helper compiled pages import from state.js,
not only ReflexEvent: a Var operation inside a callback compiles to pyMod,
isTrue, pyStrLength and so on. With a fixed six-name list those throw
ReferenceError and the result never reaches the backend.

The module imports itself and scriptScope() spreads its exports in front
of the per-call addEvents, socket and token, so both evaluators share one
parameter list built from the real export set. The typeof guards in
createExecutionContext go away with it. The node-snippet tests stub
stateExports, both vm harnesses link the self-import, and the README
describes the script scope.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

View guided diff | Re-trigger cubic

Comment thread packages/reflex-frontend/README.md Resolved
One Function-constructor evaluator covers callbacks, _call_script and
_call_function: its body is a strict eval of the code parameter, so
expressions and statement lists both keep their completion value, and no
expression-first attempt is needed. It is compiled on the first script and
takes every export of state.js plus addEvents, socket, navigate, params and
token.

The vitest file pins the contract in eight cases: completion value, every
export visible, the four locals, strict mode and isolation, lazy compile,
remount, string _call_function. The node-snippet template tests go; the
call-site test checks the runScript sites declare the addEvents alias. The
news fragment states the user-visible effect: a smaller shared chunk.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 9 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

@FarhanAliRaza

Copy link
Copy Markdown
Contributor

I pushed three commits on top of your branch. Here is what each one changes and why.

Your version (up to 6759608). It replaced the direct eval in state.js with two Function-constructor evaluators:

  • evaluateInExplicitEnvironment tries the code as an expression first. If that is a SyntaxError, it falls back to executeScriptInExplicitEnvironment, which uses a strict eval in a lazily built Function.
  • Both get a fixed set of six names (addEvents, ReflexEvent, applyEventActions, refs, socket, token) from createExecutionContext, which has typeof guards.
  • evalReactComponent uses a native import().

The approach fixes #7860. The direct eval is gone, so the minifier can mangle the shared chunk.

c868dde: pass every state.js export to scripts.

  • Problem: a fixed name list breaks real callbacks. A Var operation inside a callback compiles to a helper that compiled pages import from state.js, such as pyMod, isTrue or pyStrLength. With six names, those throw ReferenceError, and the result never reaches the backend.
  • Change: state.js imports itself (import * as stateExports from "$/utils/state"), and the scripts receive all of its exports. The list now follows the real export set, so a new helper cannot drift out of it. The typeof guards went away, because those names are always defined in the module. The two vm test harnesses link the self-import.

0c2cc28: replace the two evaluators with one runScript helper.

  • A strict eval of the code inside the Function body already returns the completion value for an expression and for a statement list. The expression-first attempt and its SyntaxError fallback added a second code path but no behavior. One evaluator covers callbacks, _call_script and _call_function.
  • The evaluator is still built on first use. A CSP without unsafe-eval rejects the Function constructor, so apps that never send scripts must still load.
  • It also passes navigate and params again. On main, the direct eval saw them, and apps written against it can use them. So the scope is every export plus addEvents, socket, navigate, params and token.
  • Tests: dynamic_execution.test.js (633 lines) became run_script.test.js (122 lines). Its eight cases pin these behaviors: completion value, every export visible, the four per-call names, strict mode and isolation, lazy compilation, the event-loop remount, and a string _call_function. The node-snippet tests in test_state_js_template.py went too. The remaining test checks that each runScript call site declares the addEvents alias first.
  • The feature fragment became a performance fragment, because the user-visible effect is the smaller shared chunk.
  • Result: the PR diff against main went from about +909 to about +220/−78. The minified chunk size is the same as with your version.

425e405: correct the news fragment. The fragment said scripts see "only addEvents, ReflexEvent, applyEventActions and the globals". After c868dde that was wrong. It now says that scripts see every export plus the per-call names, and that only names the module does not export (its imports) are out of scope.

I also updated the PR description to match the current code.

@FarhanAliRaza FarhanAliRaza added the enhancement Anything you want improved label Nov 13, 2026 — with ChatGPT Codex Connector
@FarhanAliRaza
FarhanAliRaza merged commit 18d20ac into reflex-dev:main Nov 13, 2026
182 of 185 checks passed
FarhanAliRaza added a commit that referenced this pull request Nov 13, 2026
The state.js harnesses link two special cases now: main's self-import of
$/utils/state for server-provided scripts (#7870), and the real
event_results.js module.

Claude-Session: https://claude.ai/code/session_01LwJUCSPGJ9cvdFM2KLFHoH
@somiljain2006
somiljain2006 deleted the Eval branch November 14, 2026 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Anything you want improved on deck PRs lined up to review / merge next

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Direct eval in utils/state.js stops minification of the shared chunk: ~72 KB extra JS with frontend_lazy_bundled_libraries

3 participants