Skip to content

docs: corrections and pitfalls from AI builder field notes#6775

Draft
amsraman wants to merge 1 commit into
mainfrom
docs/builder-field-notes-pitfalls
Draft

docs: corrections and pitfalls from AI builder field notes#6775
amsraman wants to merge 1 commit into
mainfrom
docs/builder-field-notes-pitfalls

Conversation

@amsraman

Copy link
Copy Markdown
Contributor

What

Five doc pages get corrections and pitfall callouts sourced from Reflex Build's curated knowledge base — rules our AI builder learned from real failures that turned out to be gaps (and in one case an error) in these pages. We're consolidating that knowledge base into the official docs so users and agents read the same source of truth (flexgen ENG-10044).

  • library/forms/upload.md — bug fix: rx.get_upload_url was documented as returning str; it returns a frontend Var and is unusable in backend handlers/computed vars (backend alternative noted). Plus: upload id must be a string literal, file.name note, handlers must not be background=True, the upload trigger button must live outside the dropzone, a short "Uploading from a Form" subsection (on_submit never carries file data), and don't hardcode /_upload/ paths.
  • enterprise/drag-and-drop.md@rx.memo params are Vars (index / rx.foreach, no Python loops); on_drag_start doesn't exist; nested drop targets fire once per layer (dedupe) + root-level drop pattern; assign collected_params to a variable before reading fields, with a _collected_params=..._replace(_js_expr=...) example for multiple zones in one memo component; warning against adding a second DnD provider.
  • state_structure/shared_state.md — sanitize tokens derived from query params (underscore restriction); _link_to pitfalls (must await, use the returned instance, only exists on rx.SharedState); computed vars depending only on shared vars belong on the shared state.
  • utility_methods/lifespan_tasks.md — a "Lifespan Tasks vs Background Events" decision section.
  • library/typography/markdown.md — literal $ pairs are parsed as LaTeX math (prices vanish, notably in streamed LLM output); escape before rx.markdown.

All additions follow each page's existing conventions (md alert callouts, demo blocks, doc voice). No behavioral claims were included that we couldn't verify against current reflex (e.g. we dropped a stale note about file.filename — it exists as an alias of .name).

🤖 Generated with Claude Code

Upstreams doc gaps found while reconciling Reflex Build's curated
knowledge base against these pages:

- upload: rx.get_upload_url returns a frontend Var, not str (bug fix);
  upload id must be a literal; handler/trigger placement pitfalls;
  uploading-from-a-form guidance; avoid hardcoding /_upload/ paths
- drag-and-drop: memo params are Vars; on_drag_start unsupported;
  nested drop dedupe + root-level drops; collected-params naming with
  _collected_params example; don't add a second provider
- shared_state: token sanitization; _link_to pitfalls; computed var
  placement
- lifespan_tasks: lifespan task vs background event decision guidance
- markdown: escaping literal dollar signs (LaTeX delimiter)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@amsraman amsraman requested review from a team and Alek99 as code owners July 15, 2026 18:07
@amsraman amsraman marked this pull request as draft July 15, 2026 18:10
@codspeed-hq

codspeed-hq Bot commented Jul 15, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing docs/builder-field-notes-pitfalls (021e99e) with main (65a2889)

Open in CodSpeed

Footnotes

  1. 8 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 021e99e9f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

- **Type**: Returns `str`
- **Type**: Returns a frontend `Var` (a JavaScript expression), not a Python `str`

Because it returns a frontend Var, `rx.get_upload_url` is frontend-only: use it in component code and event triggers, not in backend event handlers or computed vars, where the value would not resolve to a usable string. If you genuinely need the upload URL as a plain string on the backend, construct it from `self.router.headers.host` instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use the configured upload endpoint for backend URLs

When users follow this backend fallback in deployments with backend_path or a separately served API, building from self.router.headers.host omits the configured upload prefix and scheme that get_upload_url gets from getBackendURL(env.UPLOAD) and that the server mounts via config.prepend_backend_path(str(Endpoint.UPLOAD)). This can produce links that work locally but 404 in prefixed/split frontend-backend deployments; point backend-only examples at the configured upload endpoint (for example Endpoint.UPLOAD.get_url() plus the filename) instead of the Host header.

Useful? React with 👍 / 👎.

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR backfills five documentation pages with pitfall callouts and corrections sourced from Reflex Build's AI builder knowledge base, consolidating hard-won field notes into the official docs.

  • upload.md: Fixes the documented return type of rx.get_upload_url (was str, is a frontend Var), and adds warnings for upload id constraints, background=True misuse, trigger-button placement, form+upload interaction, and path hardcoding. The backend URL construction hint is incomplete.
  • drag-and-drop.md: Adds @rx.memo Var semantics note, on_drag_start absence clarification, nested drop-target deduplication, collected_params assignment pattern, a multi-zone _replace(_js_expr=...) example (internal API, no caveat), and a duplicate-provider warning.
  • markdown.md: Adds a $-as-LaTeX-delimiter warning with a correct escape recipe.
  • shared_state.md: Adds token sanitization, _link_to pitfall callout, and a computed-var placement guideline.
  • lifespan_tasks.md: Adds a "Lifespan Tasks vs Background Events" decision section.

Confidence Score: 4/5

Safe to merge — all changes are documentation-only and correct real gaps or errors with no behavioral changes to library code.

The content is accurate across all five pages and the new warnings address real developer footguns. Two spots need a closer look: the backend URL construction hint in upload.md points to self.router.headers.host without showing how to assemble a full URL, which could leave developers with a bare hostname; and the _replace(_js_expr=...) pattern in drag-and-drop.md relies on private Var internals without any stability caveat, which could silently break after an upgrade.

docs/library/forms/upload.md (backend URL hint) and docs/enterprise/drag-and-drop.md (internal API usage)

Important Files Changed

Filename Overview
docs/library/forms/upload.md Corrects rx.get_upload_url return type (str to frontend Var), adds pitfall callouts for upload id constraints, background=True misuse, trigger button placement, form+upload interaction, and path hardcoding. Backend URL construction hint is incomplete.
docs/enterprise/drag-and-drop.md Adds memo/Var clarification, on_drag_start absence note, nested drop-target deduplication guidance, collected_params assignment pattern, and multi-zone _replace(_js_expr=...) example using internal API without caveat; also adds duplicate-provider warning.
docs/library/typography/markdown.md Adds a warning about $ being parsed as LaTeX math delimiter with a correct escape recommendation; accurate and well-placed.
docs/state_structure/shared_state.md Adds token sanitization guidance, _link_to pitfall callout, and a note about placing computed vars on the shared state. All accurate and well-structured.
docs/utility_methods/lifespan_tasks.md Adds a clear Lifespan Tasks vs Background Events decision section with concrete examples; accurate, well-placed, and consistent with existing doc style.

Reviews (1): Last reviewed commit: "docs: corrections and pitfalls from AI b..." | Re-trigger Greptile

- **Type**: Returns `str`
- **Type**: Returns a frontend `Var` (a JavaScript expression), not a Python `str`

Because it returns a frontend Var, `rx.get_upload_url` is frontend-only: use it in component code and event triggers, not in backend event handlers or computed vars, where the value would not resolve to a usable string. If you genuinely need the upload URL as a plain string on the backend, construct it from `self.router.headers.host` instead.

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.

P2 Incomplete backend URL construction hint

The note says "construct it from self.router.headers.host", but self.router.headers.host only gives the Host header value (e.g., localhost:8000 or example.com). To get a usable URL string a developer would also need the protocol (http/https) and the /_upload/<filename> path. As written, a reader who follows the hint literally might write url = self.router.headers.host and end up with a bare hostname rather than a complete URL. Consider expanding the example or at least noting that the host value must be assembled with a scheme and path prefix.

Comment on lines +364 to +382
```md alert warning
# Assign collected params to a variable first

Do not read fields directly off the class (e.g. `rxe.dnd.DropTarget.is_over`). Assign `collected_params` to a variable, then access fields through it: `params = rxe.dnd.DropTarget.collected_params`, then `params.is_over`.
```

When a single `@rx.memo` component renders more than one draggable or drop target, give each its own collected params name to avoid JavaScript name collisions. Use `_replace(_js_expr=...)` to create a uniquely named params Var and pass it via `_collected_params`:

```python
@rx.memo
def dual_drop_zones() -> rx.Component:
left_params = rxe.dnd.DropTarget.collected_params._replace(_js_expr="leftZoneParams")
right_params = rxe.dnd.DropTarget.collected_params._replace(_js_expr="rightZoneParams")
return rx.hstack(
rxe.dnd.drop_target(
rx.box("Left", bg=rx.cond(left_params.is_over, "green.100", "gray.100")),
accept=["MyDraggable"],
_collected_params=left_params,
),

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.

P2 Documents a private internal API without a stability caveat

_replace(_js_expr=...) relies on Var._replace and the private _js_expr attribute, both of which are internal Reflex APIs with no stability guarantees. Documenting them without a note like "this uses an internal API and may change" could leave users with broken code after an upgrade with no clear error message pointing back to this pattern. Consider adding a brief caveat so readers know they are opting into something that could require updating when Reflex's Var internals evolve.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines 337 to 338
- `item`: Data object passed to drop handlers
- `on_end`: Called when drag operation ends

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.

P2 Potentially misleading redirect from on_drag_start

The note says "use on_end to react to the completion of a drag operation", but that only covers the end-of-drag scenario. A developer explicitly looking for a way to react to the start of a drag will read this and still have no solution — on_end fires on completion, not on pickup. If there is no way to detect drag start at all, it would be clearer to say that explicitly rather than redirecting to an event that handles a different moment in the lifecycle.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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.

1 participant