feat: get locator working in local dev fake starter#1156
Merged
mkilpatrick merged 6 commits intomainfrom Apr 14, 2026
Merged
Conversation
Contributor
WalkthroughThis PR centralizes local-development detection by adding Sequence Diagram(s)mermaid Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 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: 2
🧹 Nitpick comments (3)
starter/src/ve.config.tsx (1)
36-38: Consider using a more specific type forlocatorComponentRegistry.The
mainComponentRegistryusesConfig<DevProps>for type safety, butlocatorComponentRegistryusesConfig<any>. SincelocatorConfigis typed asConfig<LocatorConfigProps>, you could import and use that type for consistency.💡 Suggested type improvement
import { DirectoryCategory, DirectoryCategoryComponents, DirectoryCategoryProps, MainConfigProps, locatorConfig, mainConfig, + LocatorConfigProps, } from "@yext/visual-editor"; // ... -export const locatorComponentRegistry: Record<string, Config<any>> = { +export const locatorComponentRegistry: Record<string, Config<LocatorConfigProps>> = { dev: locatorConfig, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/ve.config.tsx` around lines 36 - 38, locatorComponentRegistry is typed as Record<string, Config<any>) which loses type safety; replace the any with the specific LocatorConfigProps type used by locatorConfig. Import or reference LocatorConfigProps and change the registry type to Record<string, Config<LocatorConfigProps>> (or the same pattern used by mainComponentRegistry) so locatorComponentRegistry and locatorConfig share the precise Config generic and provide compile-time validation.packages/visual-editor/src/internal/hooks/useMessage.test.ts (1)
88-95: Good test coverage for the new behavior, consider adding a non-fake-starter case.The test correctly validates that
getOriginsForSendingreturns onlywindow.location.originwhen on a fake starter local dev route. For completeness, consider adding a test case verifying the default behavior when not on a fake starter route (i.e., that the originaltargetOriginsarray is returned/used).💡 Suggested additional test case
describe("getOriginsForSending", () => { it("uses the current origin only on fake starter local dev routes", () => { window.history.replaceState({}, "", "/dev-locator/example"); expect(getOriginsForSending(["https://dev.yext.com"])).toEqual([ window.location.origin, ]); }); + + it("includes target origins on non-fake-starter routes", () => { + window.history.replaceState({}, "", "/edit"); + + const result = getOriginsForSending(["https://dev.yext.com"]); + expect(result).toContain("https://dev.yext.com"); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/internal/hooks/useMessage.test.ts` around lines 88 - 95, Add a complementary unit test for getOriginsForSending that verifies non-fake-starter behavior: set window.history to a non-fake-starter path (not matching "/dev-locator/*"), call getOriginsForSending with a sample targetOrigins array (e.g., ["https://dev.yext.com"]), and assert that it returns the original targetOrigins array unchanged; reference getOriginsForSending to locate where to add the test and mirror the existing test structure for setup and teardown of window.history.starter/localData/dev-locator-stream__en__8967748.json (1)
7-7: The serialized layout JSON appears valid, but consider extracting to a separate file for maintainability.The embedded JSON string in
__.layoutis difficult to read and maintain inline. For local development mock data, consider storing the layout configuration in a separate JSON file and referencing it, or at minimum adding a comment explaining the structure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/localData/dev-locator-stream__en__8967748.json` at line 7, The inline serialized JSON assigned to "layout" is hard to maintain; extract the large layout string into a separate JSON file (e.g., a dev fixture) and replace the inline value with a lightweight reference or loader. Locate the "layout" key in this file and move the object containing root props (version/title/description), content/MainContent (id MainContent-b6c58f1c-102d-46af-a58c-65751efa30da) and the Locator config (id Locator-2ae506f4-a3ee-46ea-b5f9-e4c3236243a7, mapStyle, filters, resultCard, distanceDisplay) into that new file; update any readers to parse/load the external JSON and add a concise comment explaining the layout format for future devs.
🤖 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/internal/types/templateMetadata.ts`:
- Around line 79-94: The entries for emails and services in templateMetadata are
declared as scalar strings but actual fixtures return arrays; update the emails
and services objects (their field_id "emails" and "services") so their
field_type and field_type_id reflect a list type (e.g., switch field_type_id
from "type.string" to the project’s list-string identifier and adjust
field_type.variant_type to the list variant) or remove these entries from the
template metadata until list-valued locator handling is implemented; modify only
the emails and services objects in templateMetadata.ts (field_type,
field_type.variant_type, and field_type_id) to align with the fixtures.
In `@starter/src/templates/dev-locator.tsx`:
- Around line 119-128: The mock installation is happening during render
(installLocatorLocalDevMocks called at top-level), which can patch globals
without a paired cleanup if render is aborted; move the call into the existing
React.useEffect so the effect both installs and cleans up the mocks.
Specifically, remove the top-level installLocatorLocalDevMocks invocation that
sets mockCleanupRef.current, and instead inside the React.useEffect call
installLocatorLocalDevMocks(), assign its returned cleanup to
mockCleanupRef.current, and keep the existing return cleanup that calls
mockCleanupRef.current?.() and nulls it; ensure this prevents mutating
globalThis.fetch or navigator.geolocation.getCurrentPosition during render.
---
Nitpick comments:
In `@packages/visual-editor/src/internal/hooks/useMessage.test.ts`:
- Around line 88-95: Add a complementary unit test for getOriginsForSending that
verifies non-fake-starter behavior: set window.history to a non-fake-starter
path (not matching "/dev-locator/*"), call getOriginsForSending with a sample
targetOrigins array (e.g., ["https://dev.yext.com"]), and assert that it returns
the original targetOrigins array unchanged; reference getOriginsForSending to
locate where to add the test and mirror the existing test structure for setup
and teardown of window.history.
In `@starter/localData/dev-locator-stream__en__8967748.json`:
- Line 7: The inline serialized JSON assigned to "layout" is hard to maintain;
extract the large layout string into a separate JSON file (e.g., a dev fixture)
and replace the inline value with a lightweight reference or loader. Locate the
"layout" key in this file and move the object containing root props
(version/title/description), content/MainContent (id
MainContent-b6c58f1c-102d-46af-a58c-65751efa30da) and the Locator config (id
Locator-2ae506f4-a3ee-46ea-b5f9-e4c3236243a7, mapStyle, filters, resultCard,
distanceDisplay) into that new file; update any readers to parse/load the
external JSON and add a concise comment explaining the layout format for future
devs.
In `@starter/src/ve.config.tsx`:
- Around line 36-38: locatorComponentRegistry is typed as Record<string,
Config<any>) which loses type safety; replace the any with the specific
LocatorConfigProps type used by locatorConfig. Import or reference
LocatorConfigProps and change the registry type to Record<string,
Config<LocatorConfigProps>> (or the same pattern used by mainComponentRegistry)
so locatorComponentRegistry and locatorConfig share the precise Config generic
and provide compile-time validation.
🪄 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: 0d4e2b80-5565-45bf-9120-d1d00a1a1dbd
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
packages/visual-editor/src/editor/TranslatableRichTextField.tsxpackages/visual-editor/src/fields/CodeField.tsxpackages/visual-editor/src/internal/components/AdvancedSettings.tsxpackages/visual-editor/src/internal/hooks/useMessage.test.tspackages/visual-editor/src/internal/hooks/useMessage.tspackages/visual-editor/src/internal/hooks/useMessageReceivers.test.tspackages/visual-editor/src/internal/hooks/useMessageReceivers.tspackages/visual-editor/src/internal/puck/constant-value-fields/Image.tsxpackages/visual-editor/src/internal/puck/constant-value-fields/Video.tsxpackages/visual-editor/src/internal/types/templateMetadata.test.tspackages/visual-editor/src/internal/types/templateMetadata.tspackages/visual-editor/src/utils/isFakeStarterLocalDev.test.tspackages/visual-editor/src/utils/isFakeStarterLocalDev.tsstarter/localData/dev-locator-stream__en__8967748.jsonstarter/localData/mapping.jsonstarter/package.jsonstarter/src/mocks/locatorLocalDev.tsstarter/src/templates/dev-locator.tsxstarter/src/templates/dev.tsxstarter/src/ve.config.tsxstarter/tailwind.config.ts
auto-screenshot-update: true
asanehisa
previously approved these changes
Apr 14, 2026
Contributor
asanehisa
left a comment
asanehisa
left a comment
There was a problem hiding this comment.
will conflict with work brian / i did in other branch but thats ok haha
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@starter/src/mocks/locatorLocalDev.ts`:
- Around line 216-218: The current mock captures only the function value in
originalGetCurrentPosition and later restores via Object.defineProperty with
only a value, leaving an own-property shadow if the original was inherited;
instead capture the original property descriptor (use
Object.getOwnPropertyDescriptor on navigator or Geolocation.prototype as needed)
before defining the mock, and on teardown restore that full descriptor via
Object.defineProperty (or, if no descriptor existed originally, delete the
mocked own property). Update references around geolocation,
originalGetCurrentPosition, and the Object.defineProperty mock/teardown logic to
use the saved descriptor and conditional delete/restore behavior.
In `@starter/src/templates/dev-locator.tsx`:
- Around line 71-89: The current snippet replaces window.WebSocket with a bare
function (assigned in the IIFE using originalWebSocket and window.WebSocket)
which loses WebSocket.prototype and static properties (e.g., WebSocket.OPEN) and
breaks instanceof checks; update the replacement to preserve the native
constructor shape by creating a wrapper that either extends the original
constructor or returns a Proxy around originalWebSocket so prototype chain and
static constants remain intact, while intercepting the constructor call to
detect and short-circuit HMR URLs (the logic checking
url.includes('24678'|'vite'|'hmr') and the blocked-return object should remain
but must be returned only for those URLs), ensure allowed connections call
through to originalWebSocket with same signature and preserve
originalWebSocket.prototype and static members.
🪄 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: f8890515-4e00-4677-b1fe-b9010db7ac03
⛔ 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 (2)
starter/src/mocks/locatorLocalDev.tsstarter/src/templates/dev-locator.tsx
benlife5
reviewed
Apr 14, 2026
mkilpatrick
dismissed stale reviews from jwartofsky-yext and asanehisa
via
fe8273b
auto-screenshot-update: true
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
starter/src/templates/dev-locator.tsx (1)
113-120: Expose the mode toggle as a pressed button.This control behaves like a toggle, so adding
aria-pressedwill make the current state much clearer to assistive tech.Suggested fix
<button + type="button" + aria-pressed={themeMode} className={"toggle-button"} onClick={() => { setThemeMode(!themeMode); }} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/templates/dev-locator.tsx` around lines 113 - 120, The toggle button currently rendered by the JSX using themeMode and setThemeMode should expose its state to assistive tech by adding an aria-pressed attribute bound to the boolean themeMode; update the button (className "toggle-button") to include aria-pressed={themeMode} so screen readers know whether it is pressed, and keep the existing onClick handler (setThemeMode(!themeMode)) and label logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@starter/src/mocks/locatorLocalDev.ts`:
- Around line 193-202: Clamp and validate the parsed pagination values before
returning them: after parsing requestBody into parsedBody, coerce
parsedBody.offset and parsedBody.limit to numbers and enforce offset >= 0
(fallback to 0 on invalid/negative) and limit between 1 and DEFAULT_LIMIT
(fallback to DEFAULT_LIMIT on invalid/<=0/too-large); return those sanitized
offset and limit values instead of raw parsedBody values so malformed or
negative inputs cannot produce surprising slice behavior.
---
Nitpick comments:
In `@starter/src/templates/dev-locator.tsx`:
- Around line 113-120: The toggle button currently rendered by the JSX using
themeMode and setThemeMode should expose its state to assistive tech by adding
an aria-pressed attribute bound to the boolean themeMode; update the button
(className "toggle-button") to include aria-pressed={themeMode} so screen
readers know whether it is pressed, and keep the existing onClick handler
(setThemeMode(!themeMode)) and label logic intact.
🪄 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: e744b6c9-5721-4988-8f7d-7784bb209702
📒 Files selected for processing (8)
packages/visual-editor/src/internal/hooks/useMessageReceivers.tspackages/visual-editor/src/internal/types/templateMetadata.tsstarter/localData/dev-locator-stream__en__8967748.jsonstarter/src/mocks/locatorLocalDev.tsstarter/src/templates/dev-locator.tsxstarter/src/templates/dev.tsxstarter/src/utils.tsstarter/src/ve.config.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- starter/src/templates/dev.tsx
- packages/visual-editor/src/internal/hooks/useMessageReceivers.ts
- packages/visual-editor/src/internal/types/templateMetadata.ts
- starter/localData/dev-locator-stream__en__8967748.json
Comment on lines
+193
to
+202
| try { | ||
| const parsedBody = JSON.parse(requestBody) as { | ||
| offset?: number; | ||
| limit?: number; | ||
| }; | ||
|
|
||
| return { | ||
| offset: parsedBody.offset ?? 0, | ||
| limit: parsedBody.limit ?? DEFAULT_LIMIT, | ||
| }; |
Contributor
There was a problem hiding this comment.
Clamp mocked pagination values before returning them.
Malformed numeric values still flow through here. For example, negative or zero offset/limit values will produce surprising slice() behavior instead of the stable default-page fallback this mock is aiming for.
Suggested fix
try {
const parsedBody = JSON.parse(requestBody) as {
offset?: number;
limit?: number;
};
+ const offset =
+ Number.isInteger(parsedBody.offset) && parsedBody.offset >= 0
+ ? parsedBody.offset
+ : 0;
+ const limit =
+ Number.isInteger(parsedBody.limit) && parsedBody.limit > 0
+ ? parsedBody.limit
+ : DEFAULT_LIMIT;
return {
- offset: parsedBody.offset ?? 0,
- limit: parsedBody.limit ?? DEFAULT_LIMIT,
+ offset,
+ limit,
};
} catch {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| const parsedBody = JSON.parse(requestBody) as { | |
| offset?: number; | |
| limit?: number; | |
| }; | |
| return { | |
| offset: parsedBody.offset ?? 0, | |
| limit: parsedBody.limit ?? DEFAULT_LIMIT, | |
| }; | |
| try { | |
| const parsedBody = JSON.parse(requestBody) as { | |
| offset?: number; | |
| limit?: number; | |
| }; | |
| const offset = | |
| Number.isInteger(parsedBody.offset) && parsedBody.offset >= 0 | |
| ? parsedBody.offset | |
| : 0; | |
| const limit = | |
| Number.isInteger(parsedBody.limit) && parsedBody.limit > 0 | |
| ? parsedBody.limit | |
| : DEFAULT_LIMIT; | |
| return { | |
| offset, | |
| limit, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/mocks/locatorLocalDev.ts` around lines 193 - 202, Clamp and
validate the parsed pagination values before returning them: after parsing
requestBody into parsedBody, coerce parsedBody.offset and parsedBody.limit to
numbers and enforce offset >= 0 (fallback to 0 on invalid/negative) and limit
between 1 and DEFAULT_LIMIT (fallback to DEFAULT_LIMIT on
invalid/<=0/too-large); return those sanitized offset and limit values instead
of raw parsedBody values so malformed or negative inputs cannot produce
surprising slice behavior.
benlife5
approved these changes
Apr 14, 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.
No description provided.