Vite config upstream#6477
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
Greptile SummaryThis PR adds typed
Confidence Score: 3/5The Vite config renderer will emit broken JavaScript whenever a user-supplied string value contains a single quote or backslash, causing vite.config.js to fail to parse at build or dev time. The packages/reflex-base/src/reflex_base/compiler/templates.py — specifically the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["rx.Config(vite_config=..., vite_allowed_hosts=...)"] --> B["_compile_vite_config()"]
B --> C["vite_config_template()"]
C --> D["ViteConfig.__init__()"]
D --> E["Build self.default_config\n(base, plugins, build, server, resolve)"]
E --> F["ViteConfig.render(vite_config)"]
F --> G{"vite_config provided?"}
G -- Yes --> H["deepcopy(vite_config)\npop imports and functions"]
H --> I["_deep_merge(vite_config, self.default_config)\nmutates default_config in place"]
G -- No --> J["Use self.default_config as-is"]
I --> K["_render(merged_dict)"]
J --> K
K --> L["compile_imports → JS import statements"]
L --> M["Template.safe_substitute → vite.config.js string"]
M --> N["Write to .web/vite.config.js"]
Reviews (1): Last reviewed commit: "Adding some more resliency" | Re-trigger Greptile |
| ], | ||
| }, | ||
| }, | ||
| "resolve": { |
There was a problem hiding this comment.
String values not escaped before JS injection
_render wraps Python strings in single quotes without escaping ', backslash, or newline characters. Any user-supplied string value containing a literal single quote (e.g. "server": {"origin": "https://it's-example.com"}) will produce syntactically broken JavaScript: origin: 'https://it's-example.com'. A minimal fix is json.dumps(v) (which produces a double-quoted, properly-escaped JS string literal) or at least v.replace("\\", "\\\\").replace("'", "\\'") before interpolation.
| for k, v in mergee.items(): | ||
| if isinstance(v, dict) and isinstance(merger.get(k), dict): | ||
| merger[k] = self._deep_merge(v, merger[k]) | ||
| elif isinstance(v, list): | ||
| if k in merger: | ||
| merger[k].extend(v) | ||
| else: | ||
| merger[k] = v | ||
| else: | ||
| merger[k] = v | ||
| return merger |
There was a problem hiding this comment.
_deep_merge mutates self.default_config in place
merger (which is self.default_config) is modified in-place at every dict/list key without first deep-copying it. Right now ViteConfig is single-use so it doesn't cause a visible bug, but calling render() a second time on the same instance would accumulate changes from the previous call. A copy.deepcopy(self.default_config) at the start of render() (or at the top of _deep_merge) would make the method safe regardless of how many times it's called.
| Var("safariCacheBustPlugin()"), | ||
| ], | ||
| "build": { | ||
| "sourcemap": sourcemap, | ||
| "rollupOptions": { | ||
| "": Var(self._ON_WARN), | ||
| "jsx": {}, | ||
| "output": { | ||
| "advancedChunks": { | ||
| "groups": [ | ||
| {"test": Var("/env.json/"), "name": "reflex-env"} | ||
| ], | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Empty-string key convention is undocumented and a potential footgun
Using "" as a dict key to inject a raw JS expression directly into the rendered object (here for onwarn) is a creative but completely invisible contract. Nothing in ViteConfigDict or any docstring describes this mechanism, so a user who passes build.rollupOptions as a dict and accidentally includes an empty key with a non-Var value will receive an opaque RuntimeError. Consider documenting this escape hatch in the class docstring or replacing it with an explicit ViteConfig-level raw_props list.
Implemented upstream Vite config customization.
This adds a typed
vite_configoption torx.Config, renders Vite config from structured Python data, and deeply merges user-provided config with Reflex defaults. Custom config supports raw JS viaVar, extra imports, and helper functions, while preserving Reflex defaults like React Router, Safari cache busting, HMR, sourcemaps, aliases, and build options.Also marks
vite_allowed_hostsas a deprecation warning directing users tovite_config={"server": {"allowedHosts": ...}}, but still parses it for backwards compatibility.All Submissions:
Type of change
Please delete options that are not relevant.
New Feature Submission:
Changes To Core Features: