feat: support analytics for custom code section#1262
Conversation
|
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. |
commit: |
WalkthroughThis change modifies CustomCodeSection.tsx to route injected user JavaScript through a new analytics bridge and script runner. A shared window.YextCustomCodeAnalytics object maintains per-componentId track handlers in a scopes map, exposing register/unregister/track operations with a default C_CUSTOM action and warnings for untracked scopes. A new CustomCodeAnalyticsBridgeAndScriptRunner component registers this bridge, replaces the previous script-tag injection effect, wraps the user script with a yextAnalytics.track helper forwarding to the shared bridge, and cleans up the script and registration on unmount. The runner is wired into CustomCodeSectionWrapper with componentId, containerRef, processedJavascript, and scriptTagId. Sequence Diagram(s)sequenceDiagram
participant CustomCodeSectionWrapper
participant CustomCodeAnalyticsBridgeAndScriptRunner
participant WindowBridge as window.YextCustomCodeAnalytics
participant ScriptTag as Injected Script
CustomCodeSectionWrapper->>CustomCodeAnalyticsBridgeAndScriptRunner: render(componentId, processedJavascript, scriptTagId)
CustomCodeAnalyticsBridgeAndScriptRunner->>WindowBridge: register(componentId, track handler)
CustomCodeAnalyticsBridgeAndScriptRunner->>ScriptTag: remove existing script by scriptTagId
CustomCodeAnalyticsBridgeAndScriptRunner->>ScriptTag: append new script with yextAnalytics.track wrapper
ScriptTag->>WindowBridge: track(componentId, data)
WindowBridge->>WindowBridge: lookup scope handler
CustomCodeAnalyticsBridgeAndScriptRunner->>WindowBridge: unregister(componentId) on cleanup
CustomCodeAnalyticsBridgeAndScriptRunner->>ScriptTag: remove script on cleanup
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/visual-editor/src/components/customCode/CustomCodeSection.tsx`:
- Around line 205-216: The injected user script in CustomCodeSection’s script
creation is wrapped in a block, which prevents global handler resolution for
inline callbacks like onclick="changeColor(this)". Update the script injection
around processedJavascript so user code runs at top level instead of inside a
block, and provide yextAnalytics through another mechanism in the same
CustomCodeSection flow without relying on block-scoped declarations.
🪄 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: f613126d-9a2d-4221-aba2-0222a8921d7f
📒 Files selected for processing (1)
packages/visual-editor/src/components/customCode/CustomCodeSection.tsx
| // Wrap user code in a block so each component gets a local yextAnalytics helper without leaking a global. | ||
| script.text = ` | ||
| { | ||
| const yextAnalytics = { | ||
| track: (eventName, data) => | ||
| window.YextCustomCodeAnalytics.track(${JSON.stringify(componentId)}, eventName, data), | ||
| }; | ||
|
|
||
| ${processedJavascript} | ||
| } | ||
| `; | ||
| containerRef.current.appendChild(script); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the test to confirm reliance on global-scope handler resolution
fd -t f 'CustomCodeSection.test.tsx' packages/visual-editor/src/components/customCode --exec sed -n '1,80p' {}Repository: yext/visual-editor
Length of output: 2587
🏁 Script executed:
python3 - <<'PY'
import subprocess, textwrap, json, os, sys, tempfile
# Use node's vm to run code as a script in a fresh global-like context.
js = r"""
const vm = require('vm');
function run(src) {
const sandbox = { console };
vm.createContext(sandbox);
try {
vm.runInContext(src, sandbox);
} catch (e) {
return { error: String(e), sandbox };
}
return { sandbox };
}
for (const [name, src] of [
['direct function', `function changeColor() {}`],
['block function', `{ function changeColor() {} }`],
['block const func expr', `{ const changeColor = () => {} }`],
['block let class', `{ let changeColor = 1; class Foo {} }`],
]) {
const res = run(src);
const sandbox = res.sandbox || {};
console.log('---', name);
console.log('error:', res.error || null);
console.log('has changeColor in sandbox:', Object.prototype.hasOwnProperty.call(sandbox, 'changeColor'));
console.log('typeof sandbox.changeColor:', typeof sandbox.changeColor);
console.log('has Foo in sandbox:', Object.prototype.hasOwnProperty.call(sandbox, 'Foo'));
console.log('typeof sandbox.Foo:', typeof sandbox.Foo);
}
"""
subprocess.run(["node", "-e", js], check=False)
PYRepository: yext/visual-editor
Length of output: 797
Wrapping user JS in a block breaks global handler resolution.
let/const/class declarations inside the injected script stay block-scoped, so inline handlers like onclick="changeColor(this)" will fail unless the handler is a function declaration. That still works only because of sloppy-mode Annex B semantics, so the current test can miss the regression. Keep the user script at top level and inject yextAnalytics another way.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/visual-editor/src/components/customCode/CustomCodeSection.tsx`
around lines 205 - 216, The injected user script in CustomCodeSection’s script
creation is wrapped in a block, which prevents global handler resolution for
inline callbacks like onclick="changeColor(this)". Update the script injection
around processedJavascript so user code runs at top level instead of inside a
block, and provide yextAnalytics through another mechanism in the same
CustomCodeSection flow without relying on block-scoped declarations.
jhelenek
changed the title
jhelenek
requested review from
asanehisa,
benlife5,
briantstephan,
jwartofsky-yext and
mkilpatrick
|
|
||
| type AnalyticsTrackProps = Parameters< |
There was a problem hiding this comment.
I think export type TrackProps exists in pages-components if that is easier to use?
| ref={containerRef} | ||
| dangerouslySetInnerHTML={{ __html: processedHtml }} | ||
| /> | ||
| <CustomCodeAnalyticsBridgeAndScriptRunner |
There was a problem hiding this comment.
Does it not work to simply add track to the window like this?
const pagesAnalytics = useAnalytics();
useEffect(() => {
(window as any).YextCustomCodeAnalytics.track = (request) => {
pagesAnalytics?.track(request);
};
return () => {
delete (window as any).YextCustomCodeAnalytics.track;
};
}, [pagesAnalytics]);
There was a problem hiding this comment.
Wouldn't window.YextCustomCodeAnalytics.track get overwritten if we had multiple custom code components?
There was a problem hiding this comment.
True. To avoid creating a separate instance per component you can probably add it in VisualEditorProvider which wraps everything and is used in every template.
There was a problem hiding this comment.
if we move const pagesAnalytics = useAnalytics(); into VisualEditorProvider, custom-code events would not automatically be tagged with the customCodeSection scope, right?
Example tree:
<AnalyticsProvider>
<VisualEditorProvider>
<AnalyticsScopeProvider name="customCodeSection">
<CustomCodeSection />
</AnalyticsScopeProvider>
</VisualEditorProvider>
</AnalyticsProvider>
3 participants
This PR supports analytics for custom code section. It adds the Analytics track method to the window so that inner-components are able to reference it. Each analytics is registered by componentId so multiple custom code components can coexist without overwriting each other's handlers.
TEST=manual
Created a dev-release with this change and set it up with a site. For that site's layout, create 3 separate custom code components, each with different analytic calls. Deployed changes and saw analytic events come in both on the network tab and in Snowflake.
Snowflake query:
select * from prod_analytics2.public.analytics_events where original_business_id = 4146665 order by event_timestamp desc;Video:
https://drive.google.com/file/d/1SVUUV4njl_fKxF-ZfO3aDrCqPrm3PgAx/view?usp=drive_link