feat: handle single linked entity field#1167
Open
jwartofsky-yext wants to merge 21 commits intomainfrom
Open
Conversation
Also handles embedded fields
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis change introduces support for linked entity schemas throughout the visual editor. A new module Sequence DiagramsequenceDiagram
participant Bridge as Platform Bridge
participant Hook as usePlatformBridgeLinkedEntitySchemas
participant Context as LinkedEntitySchemasContext
participant Selector as YextEntityFieldSelector
participant Resolver as resolveYextEntityField
Bridge->>Hook: message: getLinkedEntitySchemas<br/>(payload with linkedEntitySchemas)
activate Hook
Hook->>Hook: deep compare<br/>with current state
Hook->>Hook: update state with<br/>received schemas
Hook->>Bridge: response: success
deactivate Hook
Selector->>Context: useLinkedEntitySchemas()
Context->>Selector: return linkedEntitySchemas
Selector->>Selector: getFieldsForSelector()<br/>(with linkedEntitySchemas)
Selector->>Selector: build combined field<br/>options (entity + linked)
Selector->>Resolver: resolveField(document,<br/>fieldPath)
activate Resolver
Resolver->>Resolver: traverse dot-separated<br/>path segments
alt is array element
Resolver->>Resolver: select [0],<br/>set traversedMultiValueReference
else missing segment
Resolver->>Resolver: return early with<br/>traversedMultiValueReference
end
Resolver->>Selector: return FieldResolution<br/>(value, flag)
deactivate Resolver
Selector->>Selector: emit warning if<br/>traversedMultiValueReference true
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/visual-editor/src/internal/types/templateMetadata.ts (1)
4-4: Optional: import the type only.
LinkedEntitySchemasis used purely as a type annotation on Line 26. Usingimport typekeeps this from contributing to the runtime module graph and also makes the internal → editor dependency type-only.♻️ Suggested tweak
-import { LinkedEntitySchemas } from "../../editor/linkedEntityFieldUtils.ts"; +import type { LinkedEntitySchemas } from "../../editor/linkedEntityFieldUtils.ts";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/internal/types/templateMetadata.ts` at line 4, Change the runtime import of LinkedEntitySchemas to a type-only import so it doesn't affect the module graph: replace the existing import of LinkedEntitySchemas with an "import type { LinkedEntitySchemas } ..." and keep the rest of the file as-is (the symbol is only used as a type annotation in templateMetadata). This ensures the dependency becomes type-only and removes any runtime impact from the import.packages/visual-editor/src/internal/types/templateMetadata.test.ts (1)
39-66: Minor: the assertion only validates object spread, notgenerateTemplateMetadata.Since
generateTemplateMetadatadoesn't populatelinkedEntitySchemas, this test is effectively asserting that{...x, linkedEntitySchemas: {...}}returns the literal you just assigned. If the intent is to lock in the type shape ofTemplateMetadata.linkedEntitySchemas, a compile-timesatisfies LinkedEntitySchemasor removing the test altogether would be more expressive. Up to you.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/internal/types/templateMetadata.test.ts` around lines 39 - 66, The test currently only verifies object spread and not the behavior of generateTemplateMetadata; update the test to either assert the type shape at compile-time using TypeScript's "satisfies LinkedEntitySchemas" on the constructed object (so linkedEntitySchemas conforms to TemplateMetadata.linkedEntitySchemas) or remove the runtime assertions entirely and replace them with a compile-time check; reference the existing generateTemplateMetadata usage and linkedEntitySchemas/TemplateMetadata.linkedEntitySchemas types when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/visual-editor/src/editor/linkedEntityFieldUtils.ts`:
- Around line 87-88: The membership check using
!!linkedEntitySchemas[referenceFieldName] can be fooled by inherited properties;
replace it with an own-property check such as
Object.prototype.hasOwnProperty.call(linkedEntitySchemas, referenceFieldName) or
Object.hasOwn(linkedEntitySchemas, referenceFieldName) to ensure only direct
keys on linkedEntitySchemas (referenced by referenceFieldName) count as
linked-field schemas.
In `@packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsx`:
- Around line 346-383: The test relies on the module-level Set
warnedLinkedEntityFieldPaths in YextEntityFieldSelector.tsx being empty; to make
the test deterministic, reset or clear that set at the start of the test (e.g.,
call warnedLinkedEntityFieldPaths.clear()) before rendering; locate the Set
export/definition in YextEntityFieldSelector.tsx (warnedLinkedEntityFieldPaths)
and either export a reset helper or import the Set in the test and call .clear()
at the top of the "warns once..." test so the warning count assertion is not
affected by prior tests.
In `@packages/visual-editor/src/editor/YextEntityFieldSelector.tsx`:
- Line 60: The module-level Set warnedLinkedEntityFieldPaths is never cleared,
causing cross-test coupling and session-wide memoization; change its lifecycle
by either exporting it (warnedLinkedEntityFieldPaths) so tests can clear it in
beforeEach, or move the Set into the EntityFieldInput/YextEntityFieldSelector
instance scope (or key it by document identity + field path) so warnings are
deduplicated per instance/document, and update any references in functions that
read/write warnedLinkedEntityFieldPaths to use the new exported or
instance-scoped Set accordingly.
In `@packages/visual-editor/src/utils/resolveYextEntityField.ts`:
- Line 41: The call to resolveField is missing an explicit generic, causing TS
to infer unknown; update the call in resolveYextEntityField so it forwards the
expected generic type to resolveField (i.e., pass the resolveYextEntityField
generic/type parameter as the type argument to resolveField) when invoking
resolveField(streamDocument, entityField.field) so that the returned .value has
the correct typed T | undefined instead of unknown; reference resolveField,
resolveYextEntityField, streamDocument, and entityField.field when making this
change.
- Around line 233-236: The catch block currently logs "Error in findField:" and
always returns traversedMultiValueReference: false, which drops any traversal
state set in the try; update the function (resolveField called by findField) so
that traversedMultiValueReference is declared in the outer scope (or capture the
local tracked value) and return that actual value on error, and change the
console.error label to "Error in resolveField:" (or use the function name) so
the log is accurate; locate the try/catch in resolveYextEntityField.ts around
resolveField/findField and adjust the variable scope/log text accordingly.
---
Nitpick comments:
In `@packages/visual-editor/src/internal/types/templateMetadata.test.ts`:
- Around line 39-66: The test currently only verifies object spread and not the
behavior of generateTemplateMetadata; update the test to either assert the type
shape at compile-time using TypeScript's "satisfies LinkedEntitySchemas" on the
constructed object (so linkedEntitySchemas conforms to
TemplateMetadata.linkedEntitySchemas) or remove the runtime assertions entirely
and replace them with a compile-time check; reference the existing
generateTemplateMetadata usage and
linkedEntitySchemas/TemplateMetadata.linkedEntitySchemas types when making the
change.
In `@packages/visual-editor/src/internal/types/templateMetadata.ts`:
- Line 4: Change the runtime import of LinkedEntitySchemas to a type-only import
so it doesn't affect the module graph: replace the existing import of
LinkedEntitySchemas with an "import type { LinkedEntitySchemas } ..." and keep
the rest of the file as-is (the symbol is only used as a type annotation in
templateMetadata). This ensures the dependency becomes type-only and removes any
runtime impact from the import.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 11f61b9f-971f-466f-8e88-33ee23d1ab2d
📒 Files selected for processing (9)
packages/visual-editor/src/editor/EmbeddedFieldStringInput.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.test.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.tsxpackages/visual-editor/src/editor/linkedEntityFieldUtils.tspackages/visual-editor/src/editor/yextEntityFieldUtils.tspackages/visual-editor/src/internal/types/templateMetadata.test.tspackages/visual-editor/src/internal/types/templateMetadata.tspackages/visual-editor/src/utils/resolveYextEntityField.test.tspackages/visual-editor/src/utils/resolveYextEntityField.ts
jwartofsky-yext
changed the title
commit: |
auto-screenshot-update: true
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/visual-editor/src/hooks/useLinkedEntitySchemas.tsx (1)
24-33: Consider functional setState for the equality guard.The callback closes over
linkedEntitySchemasfrom the render in which it was created.useReceiveMessagere-registers whencallbackidentity changes, so in practice this is refreshed each render, but a functionalsetLinkedEntitySchemas((prev) => isDeepEqual(receivedSchemas, prev) ? prev : receivedSchemas)is both race-safe and avoids relying on that re-registration contract.♻️ Proposed refactor
- if (!isDeepEqual(receivedSchemas, linkedEntitySchemas)) { - setLinkedEntitySchemas(receivedSchemas); - } + setLinkedEntitySchemas((prev) => + isDeepEqual(receivedSchemas, prev) ? prev : receivedSchemas + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/hooks/useLinkedEntitySchemas.tsx` around lines 24 - 33, The equality guard currently closes over linkedEntitySchemas; update the state setter in the callback passed to useReceiveMessage to use a functional update instead: call setLinkedEntitySchemas(prev => isDeepEqual(receivedSchemas, prev) ? prev : receivedSchemas). Keep the receivedSchemas computation and the isDeepEqual check, but replace the direct comparison + setLinkedEntitySchemas(receivedSchemas) with the functional set to ensure race-safety and avoid relying on the callback re-registration behavior of useReceiveMessage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/visual-editor/src/hooks/useLinkedEntitySchemas.tsx`:
- Around line 49-58: The error thrown in useLinkedEntitySchemas references a
nonexistent VisualEditorProvider; update the thrown Error message in the
useLinkedEntitySchemas hook to reference the actual provider
(LinkedEntitySchemasContext.Provider) or a matching name used in Editor.tsx so
consumers see the correct provider name when the hook is used outside the tree
(locate the useLinkedEntitySchemas function and adjust its Error string
accordingly).
---
Nitpick comments:
In `@packages/visual-editor/src/hooks/useLinkedEntitySchemas.tsx`:
- Around line 24-33: The equality guard currently closes over
linkedEntitySchemas; update the state setter in the callback passed to
useReceiveMessage to use a functional update instead: call
setLinkedEntitySchemas(prev => isDeepEqual(receivedSchemas, prev) ? prev :
receivedSchemas). Keep the receivedSchemas computation and the isDeepEqual
check, but replace the direct comparison +
setLinkedEntitySchemas(receivedSchemas) with the functional set to ensure
race-safety and avoid relying on the callback re-registration behavior of
useReceiveMessage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 24cf1ded-f7ab-4d6b-92e6-6c62af91c480
📒 Files selected for processing (9)
packages/visual-editor/src/editor/Editor.tsxpackages/visual-editor/src/editor/EmbeddedFieldStringInput.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.test.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.tsxpackages/visual-editor/src/hooks/index.tspackages/visual-editor/src/hooks/useEntityFields.test.tsxpackages/visual-editor/src/hooks/useLinkedEntitySchemas.test.tsxpackages/visual-editor/src/hooks/useLinkedEntitySchemas.tsxpackages/visual-editor/src/internal/utils/getFilteredEntityFields.ts
✅ Files skipped from review due to trivial changes (1)
- packages/visual-editor/src/hooks/useEntityFields.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/visual-editor/src/editor/YextEntityFieldSelector.tsx
- packages/visual-editor/src/editor/EmbeddedFieldStringInput.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsx (1)
358-396: Consider resettingwarnedLinkedEntityFieldPathsin abeforeEachrather than only inside this test.
resetWarnedLinkedEntityFieldPaths()andwarningToast.mockClear()are called only inside this test body. If a future test adds a render that resolvesc_linkedLocation.nameagainst a multi-value document before this test runs, the Set will already contain that path and the "called exactly once" assertion will flip to zero. Moving both resets into abeforeEach(and/or also calling them in anafterEach) removes that implicit ordering dependency.♻️ Suggested shape
describe("YextEntityFieldSelector", () => { + beforeEach(() => { + warningToast.mockClear(); + resetWarnedLinkedEntityFieldPaths(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsx` around lines 358 - 396, Move the test-local resets into a shared test setup: call resetWarnedLinkedEntityFieldPaths() and warningToast.mockClear() inside a beforeEach (and optionally afterEach) for the suite containing YextEntityFieldSelector.test.tsx so the Set of warned paths and the mock are cleared before each test run rather than only inside the "warns once..." test; locate the calls to resetWarnedLinkedEntityFieldPaths and warningToast.mockClear in the test file and relocate them into a top-level beforeEach block for that describe/it group.packages/visual-editor/src/editor/YextEntityFieldSelector.tsx (1)
61-65: PreferWeakMapkeyed bystreamDocumentto avoid retaining stale document objects.
warnedLinkedEntityFieldPathsByDocumentis a regularMapkeyed on thestreamDocumentobject reference. BecauseMapholds a strong reference to its keys, every distinctstreamDocumentidentity seen during a session is retained for the lifetime of the module — even after the document has been replaced by a new one in context. AWeakMapwould allow garbage collection once the document is no longer referenced, and it is sufficient here since you only ever look up by identity.♻️ Proposed change
-const warnedLinkedEntityFieldPathsByDocument = new Map<any, Set<string>>(); +const warnedLinkedEntityFieldPathsByDocument = new WeakMap<object, Set<string>>();Note:
WeakMaphas noclear(), soresetWarnedLinkedEntityFieldPathswould need to reassign the variable (make it alet) or re-scope it. Alternatively, keep theMapbut accept the retention trade-off.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/editor/YextEntityFieldSelector.tsx` around lines 61 - 65, Replace the strong-reference Map with a WeakMap to avoid retaining stale streamDocument objects: change warnedLinkedEntityFieldPathsByDocument from a const Map to a let WeakMap keyed by streamDocument (e.g., warnedLinkedEntityFieldPathsByDocument = new WeakMap<any, Set<string>>()), update any lookups/sets to use the same symbol, and modify resetWarnedLinkedEntityFieldPaths to reassign warnedLinkedEntityFieldPathsByDocument to a fresh WeakMap (since WeakMap has no clear()) so previous document keys can be GC'd.
🤖 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/editor/YextEntityFieldSelector.test.tsx`:
- Around line 358-396: Move the test-local resets into a shared test setup: call
resetWarnedLinkedEntityFieldPaths() and warningToast.mockClear() inside a
beforeEach (and optionally afterEach) for the suite containing
YextEntityFieldSelector.test.tsx so the Set of warned paths and the mock are
cleared before each test run rather than only inside the "warns once..." test;
locate the calls to resetWarnedLinkedEntityFieldPaths and warningToast.mockClear
in the test file and relocate them into a top-level beforeEach block for that
describe/it group.
In `@packages/visual-editor/src/editor/YextEntityFieldSelector.tsx`:
- Around line 61-65: Replace the strong-reference Map with a WeakMap to avoid
retaining stale streamDocument objects: change
warnedLinkedEntityFieldPathsByDocument from a const Map to a let WeakMap keyed
by streamDocument (e.g., warnedLinkedEntityFieldPathsByDocument = new
WeakMap<any, Set<string>>()), update any lookups/sets to use the same symbol,
and modify resetWarnedLinkedEntityFieldPaths to reassign
warnedLinkedEntityFieldPathsByDocument to a fresh WeakMap (since WeakMap has no
clear()) so previous document keys can be GC'd.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 31627718-17c5-4143-9857-5cbf853a796b
⛔ Files ignored due to path filters (3)
packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] latest version default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] latest version multi-pageset default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] latest version default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (5)
packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.tsxpackages/visual-editor/src/editor/linkedEntityFieldUtils.tspackages/visual-editor/src/hooks/useLinkedEntitySchemas.tsxpackages/visual-editor/src/utils/resolveYextEntityField.ts
✅ Files skipped from review due to trivial changes (1)
- packages/visual-editor/src/editor/linkedEntityFieldUtils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/visual-editor/src/hooks/useLinkedEntitySchemas.tsx
- packages/visual-editor/src/utils/resolveYextEntityField.ts
auto-screenshot-update: true
auto-screenshot-update: true
jwartofsky-yext
requested review from
asanehisa,
benlife5,
briantstephan and
mkilpatrick
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/visual-editor/src/editor/YextEntityFieldSelector.tsx (1)
61-65: TightenWeakMapkey typing and consider aMapkeyed by a primitive document identifier.Two small things on the module-level cache:
WeakMap<any, Set<string>>can be tightened toWeakMap<object, Set<string>>. Theanyloses the one type guarantee this API actually enforces at runtime (keys must be non-nullish objects).- Re-assigning the binding in
resetWarnedLinkedEntityFieldPathsworks because every read goes through the module binding, but a short comment explaining "WeakMap has no.clear(), so we swap the instance" would save a future reader the detour. Alternatively, useMapkeyed by a stable document identifier (e.g.,streamDocument.idif available) — that would also sidestep the nullish/primitive key concern flagged in the useEffect below.♻️ Proposed tweak
-let warnedLinkedEntityFieldPathsByDocument = new WeakMap<any, Set<string>>(); - -export const resetWarnedLinkedEntityFieldPaths = () => { - warnedLinkedEntityFieldPathsByDocument = new WeakMap<any, Set<string>>(); -}; +// WeakMap has no `.clear()`, so the reset helper swaps the instance. +let warnedLinkedEntityFieldPathsByDocument = new WeakMap<object, Set<string>>(); + +export const resetWarnedLinkedEntityFieldPaths = () => { + warnedLinkedEntityFieldPathsByDocument = new WeakMap<object, Set<string>>(); +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/editor/YextEntityFieldSelector.tsx` around lines 61 - 65, Tighten the module cache by changing the WeakMap key type from any to object for warnedLinkedEntityFieldPathsByDocument and update resetWarnedLinkedEntityFieldPaths accordingly; either add a short comment above resetWarnedLinkedEntityFieldPaths explaining "WeakMap has no .clear(), so we replace the instance" or replace the WeakMap with a Map keyed by a stable primitive document identifier (e.g., streamDocument.id) to avoid nullish/primitive key issues referenced in the useEffect — make the change in the declarations for warnedLinkedEntityFieldPathsByDocument and the resetWarnedLinkedEntityFieldPaths function and ensure all usages still compile with the tightened typing.
🤖 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/editor/YextEntityFieldSelector.tsx`:
- Around line 61-65: Tighten the module cache by changing the WeakMap key type
from any to object for warnedLinkedEntityFieldPathsByDocument and update
resetWarnedLinkedEntityFieldPaths accordingly; either add a short comment above
resetWarnedLinkedEntityFieldPaths explaining "WeakMap has no .clear(), so we
replace the instance" or replace the WeakMap with a Map keyed by a stable
primitive document identifier (e.g., streamDocument.id) to avoid
nullish/primitive key issues referenced in the useEffect — make the change in
the declarations for warnedLinkedEntityFieldPathsByDocument and the
resetWarnedLinkedEntityFieldPaths function and ensure all usages still compile
with the tightened typing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ad3ce722-3f3a-45ea-83b8-e7a44b683eb8
⛔ Files ignored due to path filters (1)
packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] latest version default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (4)
packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.tsxpackages/visual-editor/src/editor/linkedEntityFieldUtils.tspackages/visual-editor/src/hooks/useLinkedEntitySchemas.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/visual-editor/src/editor/linkedEntityFieldUtils.ts
- packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsx
- packages/visual-editor/src/hooks/useLinkedEntitySchemas.tsx
auto-screenshot-update: true
asanehisa
reviewed
Apr 21, 2026
Contributor
asanehisa
left a comment
asanehisa
left a comment
There was a problem hiding this comment.
where in the editor is that toast?
benlife5
reviewed
Apr 21, 2026
Contributor
benlife5
left a comment
benlife5
left a comment
There was a problem hiding this comment.
Nice! Just left a few housekeeping comments
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/visual-editor/src/editor/yextEntityFieldUtils.ts (1)
28-46:⚠️ Potential issue | 🟡 MinorFallback for
directChildrenOfignores linked entity fields.The
filteredEntityFields.length === 0fallback at lines 38-44 only re-fetches from the primaryentityFields; it never re-evaluateslinkedEntityFields. Iffilter.directChildrenOftargets a linked reference (e.g.,"c_linkedLocation") whose resolved children aren't present for the current filter/types, the parent-as-list fallback won't be triggered for linked fields, producing asymmetric behavior vs. primary fields. Probably fine for the current selector usages, but worth a follow-up ifdirectChildrenOfis ever combined with linked paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/editor/yextEntityFieldUtils.ts` around lines 28 - 46, The fallback for filter.directChildrenOf only recomputes filteredEntityFields and ignores linkedEntityFields; update the logic so when filteredEntityFields is empty and you apply the allowList/types/includeListsOnly fallback you also re-run the same getFilteredEntityFields call for linkedEntityFields using buildLinkedEntityStreamFields(linkedEntitySchemas) (i.e., mirror the primary-field fallback for linkedEntityFields), ensuring linkedEntityFields is recalculated with the same { allowList: [filter.directChildrenOf], types: filter.types, includeListsOnly: true } filter so linked reference parents are handled the same as primary fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/visual-editor/src/hooks/useLinkedEntitySchemas.tsx`:
- Around line 49-58: Tests render EntityFieldInput which calls
useLinkedEntitySchemas(), but CTASelectorField.test.tsx doesn't mount
LinkedEntitySchemasContext.Provider so the hook throws; fix by wrapping the
component under test in CTASelectorField.test.tsx with
LinkedEntitySchemasContext.Provider and pass the same mock context value used in
YextEntityFieldSelector.test.tsx (the value shape expected by
useLinkedEntitySchemas/useLinkedEntitySchemasContext), so EntityFieldInput can
consume the context without error.
---
Outside diff comments:
In `@packages/visual-editor/src/editor/yextEntityFieldUtils.ts`:
- Around line 28-46: The fallback for filter.directChildrenOf only recomputes
filteredEntityFields and ignores linkedEntityFields; update the logic so when
filteredEntityFields is empty and you apply the allowList/types/includeListsOnly
fallback you also re-run the same getFilteredEntityFields call for
linkedEntityFields using buildLinkedEntityStreamFields(linkedEntitySchemas)
(i.e., mirror the primary-field fallback for linkedEntityFields), ensuring
linkedEntityFields is recalculated with the same { allowList:
[filter.directChildrenOf], types: filter.types, includeListsOnly: true } filter
so linked reference parents are handled the same as primary fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7176f907-e91c-4864-9c68-32b2fc218093
⛔ Files ignored due to path filters (1)
packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] latest version default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (5)
packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.tsxpackages/visual-editor/src/editor/linkedEntityFieldUtils.tspackages/visual-editor/src/editor/yextEntityFieldUtils.tspackages/visual-editor/src/hooks/useLinkedEntitySchemas.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/visual-editor/src/editor/linkedEntityFieldUtils.ts
auto-screenshot-update: true
auto-screenshot-update: true
asanehisa
reviewed
Apr 22, 2026
briantstephan
previously approved these changes
Apr 22, 2026
auto-screenshot-update: true
briantstephan
approved these changes
Apr 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Also handles embedded fields
If there are multiple linked entities, we use the first one and show a warning toast:
https://jam.dev/c/2f23c855-1bf6-4186-b02d-ec98358d9e8a