-
Notifications
You must be signed in to change notification settings - Fork 5.6k
[BUG] Implemented a backward-compatible solution to fix the breaking change in the Google Sheets New Row Added source #18341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] Implemented a backward-compatible solution to fix the breaking change in the Google Sheets New Row Added source #18341
Conversation
…change in the Google Sheets New Row Added source
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughUpdates Google Sheets “New Row Added” common source to always emit array Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User/Sheet Edit
participant Sheets as Google Sheets
participant Source as New Row Added Source
participant Emitter as Event Emitter
User->>Sheets: Add new row
Sheets-->>Source: Change notification/poll result
activate Source
Source->>Source: Read row, range, worksheet, rowNumber
alt hasHeaders = true
Source->>Source: Map row to object using headerRow
Note right of Source: Prepare eventData with newRow (array)<br/>+ rowAsObject (object)
else hasHeaders = false
Note right of Source: Prepare eventData with newRow (array) only
end
Source->>Emitter: emit(eventData)
deactivate Source
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/google_sheets/sources/common/new-row-added.mjs (1)
247-249: Guard against null just in case
typeof null === "object". While_transformRowToObjectwon’t return null today, add a truthy check for defensive safety.- if (this.hasHeaders && typeof rowAsObject === "object" && !Array.isArray(rowAsObject)) { + if (this.hasHeaders && rowAsObject && typeof rowAsObject === "object" && !Array.isArray(rowAsObject)) { eventData.rowAsObject = rowAsObject; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/google_sheets/package.json(1 hunks)components/google_sheets/sources/common/new-row-added.mjs(2 hunks)components/google_sheets/sources/new-row-added-polling/new-row-added-polling.mjs(1 hunks)components/google_sheets/sources/new-row-added/new-row-added.mjs(1 hunks)components/google_sheets/sources/new-row-added/test-event.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (8)
components/google_sheets/package.json (1)
3-3: Version bump looks goodPatch bump to 0.8.9 aligns with backward-compatible behavior change.
components/google_sheets/sources/new-row-added/new-row-added.mjs (1)
11-11: Source version bump OKIncrement to 0.1.16 matches the event payload adjustment in common.
components/google_sheets/sources/new-row-added-polling/new-row-added-polling.mjs (1)
11-11: Polling source version bump OK0.0.8 correctly tracks the shared behavior change.
components/google_sheets/sources/new-row-added/test-event.mjs (2)
2-7: Sample event now preserves array newRow — goodArray form for
newRowmatches the BC contract.
7-11: Adding rowAsObject is the right compatibility bridgeThis enables header-keyed access without breaking consumers of
newRowarray.components/google_sheets/sources/common/new-row-added.mjs (3)
33-33: Minor copy tweak approvedHeader row description and emphasis read clearly.
236-249: Back-compat event shape change looks correctAlways emitting
newRow(array) and conditionally addingrowAsObjectaddresses the breaking change without disrupting existing mappings.
236-249: Action: Verify there are no consumers expecting object-shapednewRow; migrate torowAsObjectif foundAutomated repo search returned no matches — manual verification required. Check components/google_sheets/sources/common/new-row-added.mjs (lines 236–249) — it now emits both the original array
newRowandrowAsObjectwhen headers are present.Run locally to locate any consumers and tests that dereference fields on
newRowand update them to useevent.rowAsObject(or preserve compatibility):
- rg -n --hidden -uu '\bnewRow\s*.' || true
- rg -n --hidden -uu '\browAsObject\b' || true
- rg -n --hidden -uu 'export default\s*{[\s\S]?"newRow"\s:\s*{' components/google_sheets || true
If any hits exist, replace usages of
newRow.<field>withevent.rowAsObject.<field>(or keep both shapes where necessary) and update docs/tests.
luancazarine
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jcortes, LGTM! Ready for QA!
WHY
Resolves #18338
Summary by CodeRabbit