Skip to content

Conversation

@iamgerwin
Copy link
Contributor

@iamgerwin iamgerwin commented Dec 11, 2025

Summary

  • Fixed the RelationSelector component to use onChange(null) instead of onChange(undefined) when clearing a single-select relation
  • Updated the onChange type signature from string | string[] | undefined to string | string[] | null
  • Updated SingleRelationInput to properly type the onChange callback

Problem

When clearing a single-select relation field in the Dashboard, the component was calling onChange(undefined). Since undefined values don't serialize in JSON, the GraphQL API would ignore the clearing request, causing previously selected values to persist on the server.

Solution

Changed onChange(undefined) to onChange(null) which properly serializes to JSON and allows the GraphQL API to receive and process the null value, effectively clearing the relation field.

Files Changed

  • packages/dashboard/src/lib/components/data-input/relation-selector.tsx - Changed handleRemove to use null instead of undefined, updated type signature
  • packages/dashboard/src/lib/components/data-input/relation-input.tsx - Updated SingleRelationInput onChange typing

Test Plan

  • Create a custom field with a single-select relation (e.g., Customer relation on Product)
  • Select a relation value
  • Click the × button to clear the selection
  • Save the form and refresh the page
  • Verify the relation is properly cleared and doesn't reappear

Closes #4032

Summary by CodeRabbit

  • Refactor
    • Updated internal handling of empty relation selection states in the dashboard for improved consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

…lation selector

The RelationSelector component's handleRemove function was calling
onChange(undefined) when clearing a single-select relation. Since
undefined values don't serialize in JSON, the GraphQL API would
ignore the clearing request, causing previously selected values
to persist.

This change uses onChange(null) instead, which properly serializes
to JSON and allows the GraphQL API to clear the relation field.

Closes vendurehq#4032
@vercel
Copy link

vercel bot commented Dec 11, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
docs Ready Ready Preview Dec 11, 2025 10:04am
vendure-storybook Ready Ready Preview Comment Dec 11, 2025 10:04am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 11, 2025

Walkthrough

This PR fixes a bug where clearing single-select relation fields passes undefined instead of null to onChange handlers, preventing proper JSON serialization and server-side clearing. The change updates RelationSelector's onChange type signature to accept null and adjusts the clear path to pass null instead of undefined.

Changes

Cohort / File(s) Summary
RelationSelector onChange contract update
packages/dashboard/src/lib/components/data-input/relation-selector.tsx
Updated RelationSelectorProps public onChange signature from (value: string | string[] | undefined) => void to (value: string | string[] | null) => void. Modified single-select clear path to pass null instead of undefined.
SingleRelationInput onChange handler
packages/dashboard/src/lib/components/data-input/relation-input.tsx
Widened onChange handler to accept string | null instead of only string, allowing null values to propagate through the component.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify the null vs. undefined change aligns with GraphQL serialization requirements and Vendure's expected input shape for optional relations
  • Confirm no other components or call sites depend on undefined behavior
  • Ensure consistent type alignment between RelationSelector and SingleRelationInput

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(dashboard): Use null instead of undefined when clearing single relation selector' clearly and concisely summarizes the primary change: switching from undefined to null in the relation selector component.
Linked Issues check ✅ Passed The PR fully addresses issue #4032 by changing onChange(undefined) to onChange(null) in RelationSelector, updating type signatures, and ensuring null values properly serialize to JSON for the GraphQL API.
Out of Scope Changes check ✅ Passed All changes are directly related to resolving the linked issue; modifications to relation-selector.tsx and relation-input.tsx are focused and scoped to the null vs undefined clearing behavior.
Description check ✅ Passed The PR description comprehensively covers the summary, problem, solution, files changed, and test plan with clear technical details about the undefined vs null serialization issue.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/dashboard/src/lib/components/data-input/relation-input.tsx (1)

39-39: Forwarding null from RelationSelector into the form engine is aligned with the fix

Handling newValue as string | null here ensures single relation fields now propagate null back to the form engine when cleared, so the outgoing JSON/GraphQL input can explicitly set the relation field to null instead of omitting it. The as string | null assertion is reasonable given multiple: false, though longer term you could consider parameterizing RelationSelector on the multiple flag to avoid these casts entirely.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 53a1d73 and 9c94bcc.

📒 Files selected for processing (2)
  • packages/dashboard/src/lib/components/data-input/relation-input.tsx (2 hunks)
  • packages/dashboard/src/lib/components/data-input/relation-selector.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/dashboard/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)

packages/dashboard/src/**/*.{ts,tsx}: Use React as the UI framework for components in the dashboard package
Use TanStack Query (useQuery or useMutation) for data fetching, not Apollo Client
Import api from '@/graphql/api.js' and graphql from '@/graphql/graphql.js' for GraphQL queries and mutations
Use the api.query() pattern with graphql document when creating useQuery calls, including queryKey and staleTime configuration
Use the api.mutate() pattern with graphql document when creating useMutation calls, with onSuccess and onError handlers
Use FormFieldWrapper component for form controls instead of raw Shadcn react-hook-form components
Use @lingui/react/macro for localization with Trans component for markup and useLingui hook for programmatic strings
Set React component props objects to Readonly<> type for type safety

Files:

  • packages/dashboard/src/lib/components/data-input/relation-selector.tsx
  • packages/dashboard/src/lib/components/data-input/relation-input.tsx
packages/dashboard/src/**/*.{ts,tsx,css}

📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)

Use Shadcn UI & Tailwind CSS for styling and UI components in the dashboard

Files:

  • packages/dashboard/src/lib/components/data-input/relation-selector.tsx
  • packages/dashboard/src/lib/components/data-input/relation-input.tsx
🧠 Learnings (7)
📚 Learning: 2025-11-27T14:21:55.972Z
Learnt from: CR
Repo: vendure-ecommerce/vendure PR: 0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-11-27T14:21:55.972Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : Set React component props objects to Readonly<> type for type safety

Applied to files:

  • packages/dashboard/src/lib/components/data-input/relation-selector.tsx
  • packages/dashboard/src/lib/components/data-input/relation-input.tsx
📚 Learning: 2025-11-27T14:21:55.972Z
Learnt from: CR
Repo: vendure-ecommerce/vendure PR: 0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-11-27T14:21:55.972Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : Import api from '@/graphql/api.js' and graphql from '@/graphql/graphql.js' for GraphQL queries and mutations

Applied to files:

  • packages/dashboard/src/lib/components/data-input/relation-input.tsx
📚 Learning: 2025-11-27T14:21:55.972Z
Learnt from: CR
Repo: vendure-ecommerce/vendure PR: 0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-11-27T14:21:55.972Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : Use FormFieldWrapper component for form controls instead of raw Shadcn react-hook-form components

Applied to files:

  • packages/dashboard/src/lib/components/data-input/relation-input.tsx
📚 Learning: 2025-11-27T14:21:55.972Z
Learnt from: CR
Repo: vendure-ecommerce/vendure PR: 0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-11-27T14:21:55.972Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : Use React as the UI framework for components in the dashboard package

Applied to files:

  • packages/dashboard/src/lib/components/data-input/relation-input.tsx
📚 Learning: 2025-11-27T14:21:55.972Z
Learnt from: CR
Repo: vendure-ecommerce/vendure PR: 0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-11-27T14:21:55.972Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : Use the api.mutate() pattern with graphql document when creating useMutation calls, with onSuccess and onError handlers

Applied to files:

  • packages/dashboard/src/lib/components/data-input/relation-input.tsx
📚 Learning: 2025-11-27T14:21:55.972Z
Learnt from: CR
Repo: vendure-ecommerce/vendure PR: 0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-11-27T14:21:55.972Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : Use the api.query() pattern with graphql document when creating useQuery calls, including queryKey and staleTime configuration

Applied to files:

  • packages/dashboard/src/lib/components/data-input/relation-input.tsx
📚 Learning: 2025-11-26T14:08:43.342Z
Learnt from: PavanendraBaahubali
Repo: vendure-ecommerce/vendure PR: 3999
File: packages/dashboard/src/app/routes/_authenticated/_products/components/create-product-variants-dialog.tsx:133-138
Timestamp: 2025-11-26T14:08:43.342Z
Learning: The Vendure dashboard codebase uses awesome-graphql-client (not Apollo Client) for GraphQL operations. The api wrapper is located at `packages/dashboard/src/vdb/graphql/api.ts`. When mutations throw errors, the error object contains a `fieldErrors` property (an array of GraphQL error objects), not `graphQLErrors`. Access error messages via `error.fieldErrors[0].message`.

Applied to files:

  • packages/dashboard/src/lib/components/data-input/relation-input.tsx
⏰ 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). (20)
  • GitHub Check: e2e tests (24.x, postgres)
  • GitHub Check: e2e tests (20.x, postgres)
  • GitHub Check: e2e tests (22.x, mariadb)
  • GitHub Check: e2e tests (24.x, mysql)
  • GitHub Check: e2e tests (22.x, sqljs)
  • GitHub Check: e2e tests (20.x, mysql)
  • GitHub Check: unit tests (20.x)
  • GitHub Check: build (22.x)
  • GitHub Check: build (24.x)
  • GitHub Check: unit tests (22.x)
  • GitHub Check: build (20.x)
  • GitHub Check: publish_install (windows-latest, 24.x)
  • GitHub Check: publish_install (macos-latest, 24.x)
  • GitHub Check: publish_install (ubuntu-latest, 24.x)
  • GitHub Check: publish_install (macos-latest, 22.x)
  • GitHub Check: publish_install (macos-latest, 20.x)
  • GitHub Check: publish_install (windows-latest, 22.x)
  • GitHub Check: publish_install (ubuntu-latest, 20.x)
  • GitHub Check: publish_install (ubuntu-latest, 22.x)
  • GitHub Check: publish_install (windows-latest, 20.x)
🔇 Additional comments (2)
packages/dashboard/src/lib/components/data-input/relation-selector.tsx (1)

51-51: Switching clear semantics from undefined to null looks correct

Changing onChange to accept string | string[] | null and using onChange(null) for single-select removal cleanly models an explicit “no selection” state, which will serialize as null in JSON and allow the GraphQL API to clear the relation instead of ignoring the field. The multi-select path remains string[]-only, so its behavior is unchanged.

Also applies to: 338-348

packages/dashboard/src/lib/components/data-input/relation-input.tsx (1)

3-3: Whitespace-only change

This line is just formatting and has no behavioral impact.

@michaelbromley
Copy link
Member

Please can you explain to me what this is fixing. Please do not use any AI assistant when answering.

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.

dashboard: Single relation selectors fail to clear: onChange(undefined) should be onChange(null)

2 participants