Skip to content

fix: dm display field name#1062

Merged
benlife5 merged 1 commit intomainfrom
fix-field-name
Feb 25, 2026
Merged

fix: dm display field name#1062
benlife5 merged 1 commit intomainfrom
fix-field-name

Conversation

@benlife5
Copy link
Contributor

No description provided.

@github-actions
Copy link
Contributor

Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 25, 2026

Walkthrough

This pull request improves type safety across the directory component utilities. The directoryChildren prop in DirectoryList is now strongly typed with specific properties (id, name, slug, optional meta with entityType.id, and optional display name fields) instead of any[]. The label resolution logic is updated to reference dm_addressCountryDisplayName and dm_addressRegionDisplayName fields. The sortAlphabetically utility function is made generic to preserve element types through sorting operations without changing runtime behavior.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether the description relates to the changeset. Add a pull request description explaining the motivation for changing the field names and the impact of these changes on the directory component.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: dm display field name' directly relates to the main changes in the pull request, which involve updating field name references from c_addressCountryDisplayName/c_addressRegionDisplayName to dm_addressCountryDisplayName/dm_addressRegionDisplayName.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-field-name

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.

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.

🧹 Nitpick comments (3)
packages/visual-editor/src/components/directory/DirectoryWrapper.tsx (1)

31-42: Consider extracting the inline directoryChildren type to a named export.

The inline shape is fairly large and is the canonical definition for what DirectoryList consumers must supply. Exporting it as a named type makes it reusable by page templates and test fixtures without duplicating the shape.

♻️ Proposed refactor
+export type DirectoryChild = {
+  id: string;
+  name: string;
+  slug: string;
+  meta?: {
+    entityType?: {
+      id: "dm_country" | "dm_region" | "dm_city";
+    };
+  };
+  dm_addressCountryDisplayName?: string;
+  dm_addressRegionDisplayName?: string;
+};
+
 export const DirectoryList = ({
   streamDocument,
   directoryChildren,
   relativePrefixToRoot,
 }: {
   streamDocument: StreamDocument;
-  directoryChildren: {
-    id: string;
-    name: string;
-    slug: string;
-    meta?: {
-      entityType?: {
-        id: "dm_country" | "dm_region" | "dm_city";
-      };
-    };
-    dm_addressCountryDisplayName?: string;
-    dm_addressRegionDisplayName?: string;
-  }[];
+  directoryChildren: DirectoryChild[];
   relativePrefixToRoot: string;
 }) => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/visual-editor/src/components/directory/DirectoryWrapper.tsx` around
lines 31 - 42, Extract the large inline type used for directoryChildren in
DirectoryWrapper.tsx into a named exported type (e.g., DirectoryChild or
DirectoryChildrenItem) and replace the inline annotation with that named type;
update the prop/type declaration for directoryChildren in DirectoryWrapper and
any references to use the new exported type so page templates and tests can
import it for reuse (ensure the exported type includes id, name, slug, optional
meta.entityType with the specific union, and the optional
dm_addressCountryDisplayName/dm_addressRegionDisplayName fields).
packages/visual-editor/src/utils/directory/utils.ts (2)

10-13: sortBy is not bounded to keyof T, partially defeating the generic.

Making the function generic but accepting sortBy: string instead of keyof T & string means callers can still pass a non-existent field and silently fall back to the ?? "" default rather than getting a compile-time error.

♻️ Proposed fix
-export const sortAlphabetically = <T extends Record<string, any>>(
-  directoryChildren: T[],
-  sortBy: string
-): T[] => {
+export const sortAlphabetically = <T extends Record<string, any>>(
+  directoryChildren: T[],
+  sortBy: keyof T & string
+): T[] => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/visual-editor/src/utils/directory/utils.ts` around lines 10 - 13,
The function sortAlphabetically<T> accepts sortBy: string which allows passing
keys that don't exist on T; change the signature to accept sortBy: keyof T &
string so callers are constrained to actual property names of T, then update any
callsites to pass a valid key (or cast explicitly if necessary) and keep the
existing fallback (the ?? "") logic intact inside sortAlphabetically to preserve
runtime safety; this ensures compile-time validation of the property while
keeping current behavior.

23-23: Array.prototype.sort mutates the caller's array.

directoryChildren?.sort(sortFn) sorts in place, silently mutating whatever was passed in. Callers may not expect this side-effect.

♻️ Proposed fix
-  return directoryChildren?.sort(sortFn);
+  return directoryChildren ? [...directoryChildren].sort(sortFn) : directoryChildren;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/visual-editor/src/utils/directory/utils.ts` at line 23, The current
return uses directoryChildren?.sort(sortFn) which mutates the input array;
change it to return a non-mutating sorted copy by making a shallow copy before
sorting (e.g. use array spread or Array.from) so callers' arrays aren't
mutated—update the expression that references directoryChildren and sortFn to
produce and return a copied, sorted array while preserving the undefined/null
behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/visual-editor/src/components/directory/DirectoryWrapper.tsx`:
- Around line 31-42: Extract the large inline type used for directoryChildren in
DirectoryWrapper.tsx into a named exported type (e.g., DirectoryChild or
DirectoryChildrenItem) and replace the inline annotation with that named type;
update the prop/type declaration for directoryChildren in DirectoryWrapper and
any references to use the new exported type so page templates and tests can
import it for reuse (ensure the exported type includes id, name, slug, optional
meta.entityType with the specific union, and the optional
dm_addressCountryDisplayName/dm_addressRegionDisplayName fields).

In `@packages/visual-editor/src/utils/directory/utils.ts`:
- Around line 10-13: The function sortAlphabetically<T> accepts sortBy: string
which allows passing keys that don't exist on T; change the signature to accept
sortBy: keyof T & string so callers are constrained to actual property names of
T, then update any callsites to pass a valid key (or cast explicitly if
necessary) and keep the existing fallback (the ?? "") logic intact inside
sortAlphabetically to preserve runtime safety; this ensures compile-time
validation of the property while keeping current behavior.
- Line 23: The current return uses directoryChildren?.sort(sortFn) which mutates
the input array; change it to return a non-mutating sorted copy by making a
shallow copy before sorting (e.g. use array spread or Array.from) so callers'
arrays aren't mutated—update the expression that references directoryChildren
and sortFn to produce and return a copied, sorted array while preserving the
undefined/null behavior.

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ac38f2 and b4816bb.

📒 Files selected for processing (2)
  • packages/visual-editor/src/components/directory/DirectoryWrapper.tsx
  • packages/visual-editor/src/utils/directory/utils.ts

@benlife5 benlife5 merged commit 223b603 into main Feb 25, 2026
24 of 25 checks passed
@benlife5 benlife5 deleted the fix-field-name branch February 25, 2026 21:24
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.

3 participants