Skip to content

Instantiator Adjustments#1478

Merged
korgon merged 6 commits into
developfrom
instantiator-timing
Apr 24, 2026
Merged

Instantiator Adjustments#1478
korgon merged 6 commits into
developfrom
instantiator-timing

Conversation

@korgon

@korgon korgon commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

This pull request significantly improves the documentation and implementation of the RecommendationInstantiator, clarifies the behavior for grouped and legacy script block integrations, and adds robust test coverage for edge cases such as SPA navigation, DOM changes, and controller cleanup. The changes ensure that controllers are managed correctly when DOM elements are added or removed, and clarify how controllers are shared or separated based on profile configuration.

Documentation Updates:

  • Added detailed documentation and examples for both grouped and legacy recommendation script block integrations, including a clear explanation that each profile entry creates a single controller and API request, regardless of how many DOM elements match its selector. [1] [2]

Behavioral Clarifications and Implementation:

  • Refactored the controller creation logic so that:
    • For grouped blocks, a single controller is created per profile entry, and all matching elements share this controller.
    • For legacy blocks, each script tag results in its own controller.
    • Per-profile DomTargeter instances ensure each controller tracks its specific elements, and controllers are only created if the source script element is still connected to the DOM (important for SPA navigation). [1] [2] [3] [4] [5]

Testing Enhancements:

  • Expanded and improved test coverage to verify:
    • Controllers are cleaned up when their target elements are removed from the DOM.
    • Controllers persist when their target elements remain in the DOM.
    • The profileCount increments correctly when DOM is replaced and retargeted.
    • SPA navigation scenarios do not result in duplicate or stale controllers.
    • Grouped block selectors matching multiple elements share a single controller, while separate profile entries (even with the same tag) create separate controllers.
    • Attachments (plugins/middleware) are properly registered before controller creation. [1] [2] [3] [4]

Other Notable Improvements:

  • Added a new config option for beacon tracking settings in the documentation.
  • Minor refactoring for clarity and maintainability, including importing createRecommendationController and improving code comments.

These changes collectively make the recommendation integration more robust, maintainable, and easier for both developers and users to understand.


Documentation and Integration Behavior:

  • Added comprehensive documentation for grouped and legacy integration types, including clear explanations and warnings about controller sharing and API requests. [1] [2]
  • Documented the new config.beacon option for beacon tracking settings.

Controller Management and SPA Support:

  • Refactored controller creation to ensure:
    • Grouped block: one controller per profile entry, shared across all matched elements.
    • Legacy block: one controller per script tag.
    • Controllers are only created if the source script element is still connected (prevents orphaned controllers on SPA navigation). [1] [2] [3] [4] [5]

Testing and Edge Case Handling:

  • Added and improved tests to verify:
    • Controller cleanup when DOM elements are removed.
    • Controller persistence when elements remain.
    • Profile count incrementing on DOM replacement.
    • SPA navigation does not cause duplicate or stale controllers.
    • Shared vs. separate controllers for grouped profiles and selectors. [1] [2] [3] [4]

Code Quality and Maintainability:

  • Improved code clarity with better comments, variable naming, and the import of createRecommendationController.

Test Setup Improvements:

  • Adjusted test setup to register attachments before controller creation for more accurate plugin/middleware testing. [1] [2]

Copilot AI review requested due to automatic review settings April 23, 2026 20:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors RecommendationInstantiator to improve SPA navigation behavior by creating more granular DomTargeter instances, tracking targeted elements, and adding controller cleanup to prevent stale controllers from lingering.

Changes:

  • Added DomTargeter.getTargetedElems() and began tracking targeted elements for non-inject targets.
  • Refactored RecommendationInstantiator to use per-profile/per-element targeters and introduced cleanupStaleControllers() for SPA DOM replacement scenarios.
  • Expanded tests for SPA navigation, controller uniqueness per element, and cleanup behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/snap-toolbox/src/DomTargeter/DomTargeter.ts Exposes targeted elements and expands tracking of targeted nodes.
packages/snap-toolbox/src/DomTargeter/DomTargeter.test.ts Adds unit coverage for getTargetedElems().
packages/snap-preact/src/Snap.tsx Adjusts RecommendationInstantiator import to type-only.
packages/snap-preact/src/Instantiators/RecommendationInstantiator.tsx Refactors instantiation/targeting logic; adds stale controller cleanup.
packages/snap-preact/src/Instantiators/RecommendationInstantiator.test.tsx Adds/updates tests for unique controllers and SPA cleanup flows.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/snap-toolbox/src/DomTargeter/DomTargeter.ts
Comment thread packages/snap-preact/src/Instantiators/RecommendationInstantiator.tsx Outdated
Comment thread packages/snap-preact/src/Instantiators/RecommendationInstantiator.tsx Outdated
Comment thread packages/snap-toolbox/src/DomTargeter/DomTargeter.ts Outdated
Comment thread packages/snap-preact/src/Instantiators/RecommendationInstantiator.tsx Outdated
Comment thread packages/snap-preact/src/Instantiators/RecommendationInstantiator.tsx Outdated
Comment thread packages/snap-preact/src/Instantiators/RecommendationInstantiator.tsx Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

chrisFrazier77
chrisFrazier77 previously approved these changes Apr 24, 2026
@korgon korgon changed the base branch from main to develop April 24, 2026 18:06
@korgon korgon dismissed chrisFrazier77’s stale review April 24, 2026 18:06

The base branch was changed.

@korgon korgon merged commit 9fa799a into develop Apr 24, 2026
5 checks passed
@korgon korgon deleted the instantiator-timing branch April 24, 2026 18:14
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