Skip to content

Fix Data Connect skill: upsert key, fetch policy, and emulator workflow#144

Open
crrapi wants to merge 1 commit into
firebase:mainfrom
crrapi:fix/data-connect-skill-accuracy
Open

Fix Data Connect skill: upsert key, fetch policy, and emulator workflow#144
crrapi wants to merge 1 commit into
firebase:mainfrom
crrapi:fix/data-connect-skill-accuracy

Conversation

@crrapi

@crrapi crrapi commented Jun 30, 2026

Copy link
Copy Markdown

What

An accuracy pass on the firebase-data-connect-basics skill. Each change fixes something an agent following the current docs hits in practice.

Changes

  • examples.md — the Movie Review schema doesn't compile. Review uses a surrogate id PK with @unique(["movie","user"]), but review_upsert keys on the primary key, so it fails with key id is required in upsert but is not set in data. Switched to a composite @table(key: ["movie","user"]) PK (which is also the one-review-per-user guarantee) and updated the dependent MyReviews / DeleteReview operations.
  • operations.md — note that _upsert matches on the primary key, not on @unique constraints.
  • sdk_web.md — document that queries default to PREFER_CACHE and that fresh/polled/live reads need SERVER_ONLY; correct the fetch-policy call shape to the { fetchPolicy } options object.
  • config.md — offline validation / SDK generation runs through the emulator with a demo- project (compile and sdk:generate need credentials); flag that the emulator returns UUIDs without hyphens; clarify outputDir is relative to connector.yaml.
  • data_seeding.md — show how to actually execute seeds against the emulator via the generated SDK (the docs never showed a runner).

Notes

Verified against firebase-tools 15.22.3 / dataconnect-emulator 3.4.14 by building a working movie-review app. scripts/format.sh --check passes; docs-only, no skill behavior contract changed.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request updates the Firebase Data Connect documentation to clarify schema design for upserts (using composite primary keys instead of surrogate IDs), local seeding via the SDK, offline emulator usage, and fetch policy options for the Web SDK. The review feedback correctly points out that generated action shortcuts do not accept fetch policy options directly and suggests updating configuration examples to use the correct relative path depth (../../) to match the new documentation.

Comment on lines +106 to +108
await executeQuery(queryRef, { fetchPolicy: QueryFetchPolicy.CACHE_ONLY });
await executeQuery(queryRef, { fetchPolicy: QueryFetchPolicy.SERVER_ONLY });
await listMovies(dataConnect, { fetchPolicy: QueryFetchPolicy.SERVER_ONLY });

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.

medium

The generated action shortcuts (such as listMovies) do not accept an options object (like { fetchPolicy }) as an argument. They only accept a DataConnect instance and/or variables. To customize the fetch policy or other execution options, you must use executeQuery with the query reference.

We should remove the incorrect listMovies example to prevent compilation errors for users following this guide.

Suggested change
await executeQuery(queryRef, { fetchPolicy: QueryFetchPolicy.CACHE_ONLY });
await executeQuery(queryRef, { fetchPolicy: QueryFetchPolicy.SERVER_ONLY });
await listMovies(dataConnect, { fetchPolicy: QueryFetchPolicy.SERVER_ONLY });
await executeQuery(queryRef, { fetchPolicy: QueryFetchPolicy.CACHE_ONLY });
await executeQuery(queryRef, { fetchPolicy: QueryFetchPolicy.SERVER_ONLY });

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Kept intentionally. The generated query shortcuts do accept ExecuteQueryOptions as the trailing argument — the SDK emits overloads listMovies(dc, options?) and recentReviews(dc, vars?, options?), and passing { fetchPolicy: SERVER_ONLY } to them is verified working on firebase 12.x (it's exactly what turns a stale cached read into a fresh one). Only mutation shortcuts (e.g. addReview) omit the options argument.

@@ -87,6 +87,10 @@ generate:
outputDir: "../ios/MyApp/DataConnect"

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.

medium

Since outputDir is resolved relative to the connector.yaml directory (typically dataconnect/connector/), targeting sibling directories at the project root requires walking up two levels (../../). To align with the helpful note added below, we should update the examples in this configuration block (including javascriptSdk and kotlinSdk above) to use ../../ instead of ../ to avoid confusing readers.

Suggested change
outputDir: "../ios/MyApp/DataConnect"
outputDir: "../../ios/MyApp/DataConnect"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Applied in 55a8cf2 — updated the javascriptSdk/kotlinSdk/swiftSdk examples in this block to ../../ so they match the note.

…lator behavior

- examples: Review must use a composite (movie, user) primary key so
  review_upsert has a conflict target; the surrogate-id + @unique version fails
  to compile. Update the dependent MyReviews/DeleteReview operations.
- operations: note that _upsert matches on the primary key, not @unique.
- sdk_web: queries default to PREFER_CACHE; pass an options object with
  SERVER_ONLY for fresh/polled reads, and fix the fetch-policy call shape.
- config: offline validation/SDK generation runs through the emulator with a
  demo project; flag that emulator UUIDs are returned without hyphens; clarify
  outputDir is relative to connector.yaml.
- data_seeding: show how to execute seeds against the emulator via the SDK.
@crrapi crrapi force-pushed the fix/data-connect-skill-accuracy branch from f7ec7e6 to 55a8cf2 Compare June 30, 2026 07:34
@mtr002 mtr002 self-assigned this Jun 30, 2026
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.

2 participants