feat(analytics-js-integrations): add Wingify device mode integration - #3056
feat(analytics-js-integrations): add Wingify device mode integration#3056zeeshan-vwo wants to merge 6 commits into
Conversation
Introduce Wingify as a new destination with V3.0 smartcode loader and full SDK registration across integrations, plugins, and legacy mappings. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review. 📝 WalkthroughWalkthroughAdds a new Wingify device-mode integration with constants, utilities, a native SDK loader, browser event and visitor handling, cross-package registration, and Jest coverage for runtime behavior. ChangesWingify Browser Integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to This PR adds the Wingify destination and related registrations with the supplied tests and checks reported as passing; no actionable merge-blocking risk remains beyond normal review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds the Wingify destination/integration across the analytics packages so it can be loaded, resolved via aliases, and displayed consistently.
Changes:
- Registered Wingify in integration registries, CNAME mappings, and client/server naming maps.
- Added the Wingify browser integration implementation, utilities, and native SDK loader.
- Added unit tests for Wingify utilities and browser behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/analytics-v1.1/src/constants/integrations/integration_cname.js | Adds Wingify to the shared CNAME mapping list used by analytics-v1.1. |
| packages/analytics-v1.1/src/constants/integrations/client_server_name.js | Adds Wingify to client-to-server destination name mapping. |
| packages/analytics-js-plugins/src/utilities/destDisplayNamesToFileNamesMap.ts | Maps Wingify display name to its directory name for plugin file resolution. |
| packages/analytics-js-legacy-utilities/src/config_to_integration_names.js | Adds Wingify config key mapping for legacy utilities. |
| packages/analytics-js-integrations/src/integrations/index.js | Registers Wingify in the integrations registry. |
| packages/analytics-js-integrations/src/integrations/Wingify/utils.js | Adds integration option lookup + event/attribute sanitizers. |
| packages/analytics-js-integrations/src/integrations/Wingify/nativeSdkLoader.js | Adds native Wingify SDK loader logic. |
| packages/analytics-js-integrations/src/integrations/Wingify/index.js | Adds the Wingify integration entrypoint. |
| packages/analytics-js-integrations/src/integrations/Wingify/constants.js | Adds Wingify constants, directory name, and CNAME mapping. |
| packages/analytics-js-integrations/src/integrations/Wingify/browser.js | Implements Wingify integration methods (init/track/identify/experiment hook). |
| packages/analytics-js-integrations/src/constants/Destinations.ts | Adds Wingify name/display-name constants. |
| packages/analytics-js-integrations/tests/integrations/Wingify/utils.test.js | Tests sanitizeName/sanitizeAttributes behavior. |
| packages/analytics-js-integrations/tests/integrations/Wingify/browser.test.js | Tests init script injection + track/identify calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/analytics-js-integrations/src/integrations/Wingify/constants.js (1)
5-10: 💤 Low valueDuplicate key in CNameMapping object.
The object contains both
[NAME](computed property) andWINGIFY(literal key), which both evaluate to the string'WINGIFY'sinceNAMEis imported asWINGIFY_NAMEwith value'WINGIFY'. This creates a duplicate object key.While JavaScript permits duplicate keys (the last definition wins) and both map to the same value, the duplication is redundant.
♻️ Optional: Remove redundant entry
const CNameMapping = { [NAME]: NAME, Wingify: NAME, wingify: NAME, - WINGIFY: NAME, };🤖 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/analytics-js-integrations/src/integrations/Wingify/constants.js` around lines 5 - 10, CNameMapping contains a redundant duplicate key: the computed property [NAME] and the literal "WINGIFY" both resolve to the same key; remove the literal "WINGIFY" entry so the mapping keeps the computed [NAME]: NAME and the other variants (Wingify, wingify) intact. Update the CNameMapping object (reference symbol CNameMapping and the constant NAME) to eliminate the redundant "WINGIFY" key and leave only unique keys.
🤖 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/analytics-js-integrations/__tests__/integrations/Wingify/browser.test.js`:
- Line 119: Rename the test description string to correct the typo: change the
test whose name is 'Vistor call with attributes' to 'Visitor call with
attributes' (update the string passed to the test(...) call in the test for the
Wingify browser integration).
- Around line 40-43: Replace the DOM-dependent assertion that uses
window.document.querySelector and script with a service-worker-safe mock-based
verification: spy on or stub document.createElement and
Element.prototype.appendChild (or the function your integration calls to insert
scripts) to capture the created element and its src, then assert that
createElement was called for 'script' and that appendChild received an element
whose src equals "https://edge.wingify.net/tag/654331.js"; ensure the test does
not directly access window.document (guard with globalThis.document when setting
spies) so it runs in both browser and service-worker environments.
In `@packages/analytics-js-integrations/src/integrations/Wingify/browser.js`:
- Around line 57-59: The condition that decides whether to call
experimentViewed() uses a non-existent property (this.experimentViewedIdentify)
so identify-only experiments don't trigger; update the guard in the constructor
or init path to check the correct flag names (this.sendExperimentTrack ||
this.sendExperimentIdentify) so experimentViewed() is invoked when either
sendExperimentTrack or sendExperimentIdentify is enabled (referencing the
this.sendExperimentTrack and this.sendExperimentIdentify properties and the
experimentViewed() method).
- Around line 118-120: The current extraction "const { traits } =
message.context || message" can drop top-level message.traits when
message.context exists but lacks traits; change the logic in the identify
handling so you prefer context.traits if present, otherwise fall back to
message.traits (and finally to {}), then pass that payload into
sanitizeAttributes to build formattedAttributes (update references near the
variables message, traits, payload, sanitizeAttributes, formattedAttributes).
- Around line 80-89: The code dereferences
_wingify_exp[expId].comb_n[variationId] before confirming the experiment object
exists; update the checks and logging to first grab a local exp =
_wingify_exp[expId] (or use optional chaining), verify exp is truthy and
exp.comb_n is an object, then only call logger.info and evaluate
exp.comb_n[variationId] and exp.type against
['VISUAL_AB','VISUAL','SPLIT_URL','SURVEY'] after those guards; adjust the
conditional around the logger.info and the if-statement in the browser.js
integration (references: _wingify_exp, expId, variationId, logger.info) so
missing/late metadata won’t throw.
---
Nitpick comments:
In `@packages/analytics-js-integrations/src/integrations/Wingify/constants.js`:
- Around line 5-10: CNameMapping contains a redundant duplicate key: the
computed property [NAME] and the literal "WINGIFY" both resolve to the same key;
remove the literal "WINGIFY" entry so the mapping keeps the computed [NAME]:
NAME and the other variants (Wingify, wingify) intact. Update the CNameMapping
object (reference symbol CNameMapping and the constant NAME) to eliminate the
redundant "WINGIFY" key and leave only unique keys.
🪄 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: f6e034a1-49de-4f68-92ae-96d65dc0f4fd
📒 Files selected for processing (13)
packages/analytics-js-integrations/__tests__/integrations/Wingify/browser.test.jspackages/analytics-js-integrations/__tests__/integrations/Wingify/utils.test.jspackages/analytics-js-integrations/src/constants/Destinations.tspackages/analytics-js-integrations/src/integrations/Wingify/browser.jspackages/analytics-js-integrations/src/integrations/Wingify/constants.jspackages/analytics-js-integrations/src/integrations/Wingify/index.jspackages/analytics-js-integrations/src/integrations/Wingify/nativeSdkLoader.jspackages/analytics-js-integrations/src/integrations/Wingify/utils.jspackages/analytics-js-integrations/src/integrations/index.jspackages/analytics-js-legacy-utilities/src/config_to_integration_names.jspackages/analytics-js-plugins/src/utilities/destDisplayNamesToFileNamesMap.tspackages/analytics-v1.1/src/constants/integrations/client_server_name.jspackages/analytics-v1.1/src/constants/integrations/integration_cname.js
Fix experiment identify init guard, safe localStorage config parsing, registry export mapping, and utils JSDoc/iteration. Co-authored-by: Cursor <cursoragent@cursor.com>
Guard experiment metadata access, fix identify traits fallback and event name validation, skip invalid FCP hide styles, and expand test coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Hi @shekhar-rudder, please review this PR. Let me know any other details are needed. |
|
Hello! This PR has been open for 20 days without any activity. Therefore, it's considered as stale and is scheduled to be closed in 10 days. If you're still working on this, please remove the 'Stale' label or add a comment to keep it open. Thanks for your contribution! |
|
@dependabot rebase |
|
Hello! This PR has been open for 20 days without any activity. Therefore, it's considered as stale and is scheduled to be closed in 10 days. If you're still working on this, please remove the 'Stale' label or add a comment to keep it open. Thanks for your contribution! |
| logger.info('loadIntegration flag is disabled'); | ||
| } | ||
|
|
||
| window.WINGIFY = window.WINGIFY || []; |
There was a problem hiding this comment.
Could we update this integration to use the documented window.Wingify global? Current Wingify examples initialize window.Wingify, so calls queued on window.WINGIFY will not reach SmartCode unless Wingify confirms an alias. Please add a contract test against window.Wingify instead of an uppercase local shim.
| } | ||
| const expId = data[1]; | ||
| const variationId = data[2]; | ||
| const experiment = window._wingify_exp?.[expId]; |
There was a problem hiding this comment.
The Wingify v3 documentation reads campaign metadata from window.Wingify.campaigns. I also verified current SmartCode bundles from VWO and Wingify expose window.Wingify.campaigns and _vwo_exp, but not _wingify_exp. This lookup therefore returns undefined, and the callback exits without sending Experiment Viewed. Could we use the documented callback fields or window.Wingify.campaigns and add an experiment callback test?
| try { | ||
| const storedConfig = localStorage.getItem(`_wingify_${account_id}_config`); | ||
| if (storedConfig) { | ||
| config = Object.assign(JSON.parse(storedConfig), config); |
There was a problem hiding this comment.
The official Async v3.0 SmartCode example lets the stored configuration override the default values. This code uses the reverse order. Was this change intentional for the RudderStack integration?
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
packages/analytics-js-integrations/src/integrations/Wingify/index.js:1
- The integrations registry predominantly consumes each integration module via a
.defaultexport. This module currently only provides a named export (Wingify), which forces special-casing in the registry. Consider exporting a default here as well (while keeping the named export) so the registry can treat Wingify like other integrations.
export { default as Wingify } from './browser';
packages/analytics-js-integrations/src/integrations/index.js:131
- This entry uses
Wingify.Wingifywhile the rest of the integration registry uses*.default. Mixing export shapes in theintegrationsmap makes the registry harder to reason about and can break depending on how the modules are bundled/transpiled. Consider aligning Wingify with the existing pattern by exposing a default export from./Wingifyand referencingWingify.defaulthere.
SENTRY: Sentry.default,
SNAP_PIXEL: SnapPixel.default,
TVSQUARED: TVSquared.default,
VWO: VWO.default,
WINGIFY: Wingify.Wingify,
GOOGLE_OPTIMIZE: GoogleOptimize.default,
| try { | ||
| const storedConfig = localStorage.getItem(`_wingify_${account_id}_config`); | ||
| if (storedConfig) { | ||
| config = Object.assign(JSON.parse(storedConfig), config); | ||
| } | ||
| } catch (e) { | ||
| // ignore invalid localStorage config | ||
| } |
| init() { | ||
| t._settings_timer = setTimeout(() => { | ||
| code.finish(); | ||
| }, this.settings_tolerance()); | ||
|
|
Summary
Introduce Wingify as a new device mode destination with the V3.0 async smartcode loader and full SDK registration across integrations, plugins, and legacy mappings.
This integration mirrors the existing VWO destination pattern, rebranded for Wingify:
Wingifyintegration underanalytics-js-integrationswithtrack,identify, and experiment-viewed supporthttps://edge.wingify.net/tag/{accountId}.jswindow.WINGIFY,window._wingify_code,_wingify_expintegrations/index.js,Destinations.ts, CDN file name map, v1.1 cname/client mappings, and legacy config-to-integration namesMotivation and context
Wingify is being introduced as a standalone destination alongside VWO. Customers configuring Wingify in RudderStack need a dedicated integration that loads the Wingify smartcode and forwards RudderStack events/traits to the Wingify SDK.
Test plan
browser.test.js,utils.test.js)analytics-js-integrationstest suite passes (986 tests)analytics-js-pluginstest suite passes (470 tests)Linear task (optional)
Linear task link
Cross Browser Tests
Please confirm you have tested for the following browsers:
Sanity Suite
Security
The integration only loads scripts from the configured Wingify CDN endpoint (
edge.wingify.net) using the customer-provided account ID, consistent with other device mode destination loaders in this repo.Summary by CodeRabbit
New Features
Tests
Chores