Skip to content

Conversation

@MatteoGabriele
Copy link
Owner

@MatteoGabriele MatteoGabriele commented Mar 13, 2025

  • Rewrite in Typescript
  • esm-only build
  • add unpkg link to es build
  • Check package.json with correct exports
  • Publish beta version
  • Check for missing features
  • create a way to easily wait for user consent (create an example)
  • Bump major version
  • Check events firing with DebugView
  • Enhance type definition with gtag jsdoc documentation
  • Add migration guide:
  • Update documentation
  • Open issues list
  • Check changelog
  • check next-script for a possible Nuxt module

Issue fixed or features implemented:

closes #607
closes #535
closes #533
closes #393
closes #615
closes #589
closes #334
closes #377
closes #566
closes #526

Summary by CodeRabbit

  • New Features

    • Introduced an automated type check step into the deployment workflow.
    • Streamlined module exports to simplify integration for developers.
    • Added comprehensive settings management for tracking configurations.
    • Implemented user consent management through a new composable function.
  • Chores

    • Upgraded the library version to 3.0.0-rc.0, updating build, lint, and testing scripts.
    • Enhanced configuration files and editor settings for consistent formatting.
    • Refactored internal APIs and consolidated tests for improved maintainability.

@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2025

Walkthrough

This pull request introduces significant changes across the CI workflow, configuration files, API modules, tests, and type definitions. A new CI step has been added for running type checks and various editor and build configuration adjustments are made via VS Code settings, tsconfig, and Vite. A large number of legacy API files and tests have been removed and replaced with a new modular API structure (with dedicated directories for config, consent, custom-map, ecommerce, event, etc.) alongside updated core modules and composables for tracking, settings, and consent. New TypeScript type definitions and tests have been added to support these improvements.

Changes

Files Change Summary
CI & Config Files
.github/workflows/deployment.yml, .vscode/settings.json, biome.json, package.json, tsconfig.json, vite.config.js
Added new CI step for type checking; updated editor indentation settings; refined file inclusion and formatting options; bumped version, updated scripts, dependencies, and build output; introduced new TypeScript compiler settings and Vite configuration with DTS and coverage options.
Removed Legacy Files
(e.g. src/add-configuration.js, src/add-routes-tracker.js, several files from src/api/*, src/attach-api.js, src/bootstrap.js, src/index.js, src/options.js, src/register-globals.js, src/router.js, src/track.js, src/utils.js; numerous test files under test/*)
Deprecated and removed old API entry points, utility functions, and tests to pave the way for a restructured modular API; legacy public entity declarations have been eliminated.
New Modular API Structure & Tests
(new files under src/api/config, src/api/consent, src/api/custom-map, src/api/ecommerce, src/api/event, src/api/exception, src/api/linker, src/api/opt, src/api/pageview, src/api/query, src/api/screenview, src/api/set, src/api/time; new central export in src/api/index.ts; corresponding test files)
Introduced a modular API with dedicated directories and clear re-exports; added new implementations for consent, ecommerce, and event handling; integrated updated functions for setting dimensions, tracking, and command queries; comprehensive new tests ensure each module behaves as expected.
Core & Composables Enhancements & Type Declarations
(new files under src/core/*, src/composables/use-consent/*, src/types/gtag.ts)
Added core modules for addGtag, createGtag, router tracking and settings management; introduced composables for managing user consent; replaced outdated type declarations (removing vue-gtag.d.ts) with a new comprehensive TypeScript type system to support improved plugin options, nonce support, and enhanced event parameters.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant A as Vue App
    participant B as bootstrap()
    participant Q as query()
    participant O as optIn()

    U->>A: Trigger enableCookies()
    A->>B: Call bootstrap()
    B-->>A: Return gtag instance
    A->>Q: Call query("event", "consent", {…})
    Q-->>A: Consent event sent
    A->>O: Call optIn()
Loading

Assessment against linked issues

Objective Addressed Explanation
Consent mode and API consent support (#607, #393)
Plugin options type declarations (#535, #533, #615, #589) Nonce support, consent event, and pageTrackerTemplate improvements appear implemented; however, the pageTrackerSkipSamePath type issue may still persist.
Analytics events and dimension configuration (#334, #566)
AutoTracking control and duplicate script prevention (#377, #526)

Poem

I’m a little rabbit, hopping through the code,
New consent and tracking now brightly bestowed.
Modular APIs sprout like carrots in a row,
With TypeScript and tests, our progress does glow.
No more bugs to nibble, our system’s in mode—
A joyful, quick hop into improvements we go! 🐇


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@MatteoGabriele MatteoGabriele changed the title feat!: version 3 feat: version 3 Mar 13, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (21)
.github/workflows/deployment.yml (1)

28-28: Remove trailing whitespace.

There is trailing whitespace on line 28 that should be removed.

-        
+
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 28-28: trailing spaces

(trailing-spaces)

src/api/set.test.ts (1)

7-8: Fix grammar in test description.

The test description has a grammatical error.

-  it("should the set command", () => {
+  it("should use the set command", () => {
src/api/refund.ts (1)

3-15: Consider documenting the RefundParams type

The type definition is well-structured, but adding JSDoc comments would help developers understand the purpose of each field and how they relate to Google Analytics refund tracking.

+/**
+ * Parameters for tracking a refund event
+ * @see https://developers.google.com/analytics/devguides/collection/ga4/refunds
+ */
type RefundParams =
  | {
+     /** The transaction ID of the refunded transaction */
      transaction_id?: string;
+     /** The monetary value of the refund */
      value?: number;
+     /** The currency code */
      currency?: string;
+     /** The items that were refunded */
      items?: Array<{
        id?: string;
        name?: string;
        quantity?: number;
        price?: number;
      }>;
    }
  | Gtag.CustomParams;
src/api/purchase.test.ts (1)

1-16: Test correctly verifies the purchase event API

The test properly verifies that the purchase function delegates to the query function with the correct parameters.

Consider adding tests for additional scenarios in the future, such as:

  • Multiple purchase parameters
  • Different transaction types
  • Edge cases with missing parameters
src/api/query.test.ts (1)

1-20: Test looks good but could use setup/teardown for improved isolation

The test correctly verifies that the query function passes events to the gtag library by checking the window's data layer. For improved test robustness, consider:

  1. Adding setup/teardown to clean the data layer between tests
  2. Mocking the window object instead of using the actual global window
+ beforeEach(() => {
+   // Reset dataLayer before each test
+   const { dataLayerName } = getSettings();
+   window[dataLayerName] = [];
+ });

This will prevent potential test interference if more test cases are added in the future.

src/settings.test.ts (1)

3-28: Tests look good but could benefit from better isolation

The tests correctly verify the default settings configuration and update functionality. For improved test isolation, consider:

  1. Adding a reset function to restore default settings between tests
  2. Verifying that unspecified settings remain unchanged when updating
+ beforeEach(() => {
+   // Reset to default settings before each test
+   resetSettings();
+ });

Also, consider expanding the second test to verify that other settings remain unchanged:

  expect(getSettings()).toEqual(
    expect.objectContaining({
      tagId: "UA-1234567",
      gtagName: "foo",
+     // Verify other settings remain unchanged
+     resourceUrl: "https://www.googletagmanager.com/gtag/js",
+     dataLayerName: "dataLayer",
    }),
  );
src/api/config.test.ts (1)

5-40: Tests look good but need improved test isolation

The tests correctly verify that the config function works as expected with both single and multiple accounts. For improved test isolation:

  1. Reset settings between tests
  2. Reset mock call history before each test
+ beforeEach(() => {
+   resetSettings();
+   vi.clearAllMocks();
+ });

This ensures that each test starts with a clean state and makes verification more reliable.

src/api/query.ts (1)

6-11: Consider adding JSDoc comments.

The function would benefit from documentation explaining its purpose, parameters, and return value.

+/**
+ * Executes a gtag command with the provided arguments
+ * @param args - The arguments to pass to the gtag function
+ */
export default function query(...args: QueryParams) {
  const { dataLayerName, gtagName } = getSettings();

  if (isServer()) {
    return;
  }
}
src/index.ts (1)

6-13: Consider stronger typing for the global Window interface.

While the biome-ignore comments are necessary to bypass linting, consider using more specific types than any | any[] for better type safety and intellisense support.

declare global {
  interface Window {
    // biome-ignore lint/suspicious/noExplicitAny:
-   [gtag: Settings["gtagName"]]: any | any[];
+   [gtag: Settings["gtagName"]]: (...args: any[]) => void | any[];
    // biome-ignore lint/suspicious/noExplicitAny:
-   [dataLayer: Settings["dataLayerName"]]: any | any[];
+   [dataLayer: Settings["dataLayerName"]]: any[][] | undefined;
  }
}
src/add-configuration.test.ts (1)

60-75: LGTM on the linker test.

This test correctly verifies the linker configuration is properly set before initializing the tracking.

Consider adding a test case for when tagId is not provided to verify the early return behavior.

src/add-router.tracking.test.ts (1)

1-56: Well-structured tests for router tracking functionality

The tests cover both initial tracking (when router is ready) and tracking after navigation, which is comprehensive. The use of expect.objectContaining() for partial matching in assertions is a good practice.

However, there's a discrepancy in naming - the file is named add-router.tracking.test.ts (with a dot) while it imports from add-router-tracking (with a hyphen).

Consider updating the file name to follow the same naming convention as the module it's testing:

- src/add-router.tracking.test.ts
+ src/add-router-tracking.test.ts
src/api/screenview.ts (2)

12-24: Type handling looks robust, but could be improved

The function correctly handles different parameter types with proper type guards. However, the type assertion as string on line 20 could be avoided with better type narrowing.

- template.screen_name = (params.name ?? params.path) as string;
+ template.screen_name = params.name ? String(params.name) : String(params.path);

25-27: Unnecessary optional chaining

Since template is always defined (initialized on line 15), the optional chaining in template?.app_name is unnecessary.

- if (pageTracker?.appName && template?.app_name === undefined) {
+ if (pageTracker?.appName && template.app_name === undefined) {
src/track-route.ts (1)

6-16: Redundant optional chaining in exclusion check

There's an unnecessary optional chaining in the exclude check on line 13, since pageTracker?.exclude is already checked on line 9.

- return pageTracker.exclude?.some(({ name, path } = {}) => {
+ return pageTracker.exclude.some(({ name, path } = {}) => {
src/utils.ts (2)

36-41: Consider enhancing error handling for script loading

While the basic error handling works, it would be helpful to provide more detailed error information when script loading fails.

- script.onerror = reject;
+ script.onerror = (event) => {
+   reject(new Error(`Failed to load script from ${url}: ${event}`));
+ };

5-41: Consider handling duplicate script injections

The current implementation doesn't check if a script with the same URL is already loaded or being loaded, which could lead to duplicate injections.

export async function injectScript(
  url: string,
  options?: {
    preconnectOrigin?: string;
    defer?: boolean;
  },
): Promise<void> {
  return new Promise((resolve, reject) => {
    if (isServer()) {
      return;
    }

+   // Check if script is already loaded or loading
+   const existingScript = document.querySelector(`script[src="${url}"]`);
+   if (existingScript) {
+     // If the script is already loaded, resolve immediately
+     if (existingScript.hasAttribute('data-loaded')) {
+       resolve();
+       return;
+     }
+     
+     // If the script is still loading, wait for it
+     existingScript.addEventListener('load', () => resolve());
+     existingScript.addEventListener('error', reject);
+     return;
+   }

    const head = document.head;
    const script = document.createElement("script");

    script.async = true;
    script.src = url;
+   script.setAttribute('data-loading', 'true');

    if (options?.defer) {
      script.defer = true;
    }

    if (options?.preconnectOrigin) {
      const link = document.createElement("link");

      link.href = options.preconnectOrigin;
      link.rel = "preconnect";

      head.appendChild(link);
    }

    head.appendChild(script);

-   script.onload = () => resolve();
+   script.onload = () => {
+     script.setAttribute('data-loaded', 'true');
+     script.removeAttribute('data-loading');
+     resolve();
+   };
    script.onerror = reject;
  });
}
src/api/pageview.ts (1)

21-56: Consider improving type safety for the Route interface

The implementation handles different input types well, but there's a type assertion on line 38 (params.name as string) that could be avoided with a more precise type definition for the Route interface.

Consider updating the Route interface in src/types.ts to explicitly define the name property as a string or string | undefined to avoid the need for type assertion:

// In src/types.ts
export interface Route {
  path: string;
  fullPath: string;
+  name: string | undefined;
  // other properties...
}
src/track-route.test.ts (3)

55-67: Consider adding URL origin verification for page_location

The test properly validates pageview tracking, but it assumes that page_location will always be "http://localhost:3000/about". In a real browser environment, the origin might differ.

Consider making the test more robust by using a more generic verification:

  expect(query).toHaveBeenCalledWith("event", "page_view", {
    page_path: "/about",
-   page_location: "http://localhost:3000/about",
+   page_location: expect.stringContaining("/about"),
    page_title: "about",
    send_page_view: true,
  });

69-99: Well-structured exclusion tests

The tests for route exclusion properly verify that tracking doesn't happen for excluded routes, testing both path and name-based exclusions.

Consider adding a test for combined exclusion criteria (both path and name) to ensure the exclusion logic handles multiple conditions properly.


181-201: Consider adding test for error handling

The test suite covers the happy path thoroughly but doesn't test error scenarios.

Consider adding tests for error handling, such as:

  1. What happens when trackRoute is called with an invalid route
  2. What happens when a template function throws an error
  3. How errors in callbacks are handled

This would provide more confidence in the robustness of the tracking functionality.

src/settings.ts (1)

49-50: Consider using a more robust state management approach

The current implementation uses a mutable variable to store settings, which could potentially lead to unexpected behavior if accessed or modified directly.

Consider using a more robust approach such as:

  1. Using a private variable with getter/setter methods
  2. Implementing a reactive store (like Vue's reactive)
  3. Using a singleton pattern with controlled access
-let settings: Settings = { ...defaultSettings };
+const settingsState = {
+  _settings: { ...defaultSettings } as Settings,
+  get settings(): Settings {
+    return { ...this._settings };
+  },
+  reset(): void {
+    this._settings = { ...defaultSettings };
+  },
+  update(configParams: Partial<Settings>): void {
+    this._settings = { ...this._settings, ...configParams };
+  }
+};
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1628ae9 and f6d6582.

⛔ Files ignored due to path filters (3)
  • package-lock.json is excluded by !**/package-lock.json
  • test/__snapshots__/options.spec.js.snap is excluded by !**/*.snap
  • test/__snapshots__/utils.spec.js.snap is excluded by !**/*.snap
📒 Files selected for processing (97)
  • .github/workflows/deployment.yml (1 hunks)
  • .vscode/settings.json (1 hunks)
  • biome.json (1 hunks)
  • package.json (3 hunks)
  • src/add-configuration.js (0 hunks)
  • src/add-configuration.test.ts (1 hunks)
  • src/add-configuration.ts (1 hunks)
  • src/add-gtag.test.ts (1 hunks)
  • src/add-gtag.ts (1 hunks)
  • src/add-router-tracking.ts (1 hunks)
  • src/add-router.tracking.test.ts (1 hunks)
  • src/add-routes-tracker.js (0 hunks)
  • src/api/config.js (0 hunks)
  • src/api/config.test.ts (1 hunks)
  • src/api/config.ts (1 hunks)
  • src/api/custom-map.js (0 hunks)
  • src/api/custom-map.test.ts (1 hunks)
  • src/api/custom-map.ts (1 hunks)
  • src/api/disable.js (0 hunks)
  • src/api/event.js (0 hunks)
  • src/api/event.test.ts (1 hunks)
  • src/api/event.ts (1 hunks)
  • src/api/exception.js (0 hunks)
  • src/api/exception.test.ts (1 hunks)
  • src/api/exception.ts (1 hunks)
  • src/api/index.js (0 hunks)
  • src/api/index.ts (1 hunks)
  • src/api/linker.js (0 hunks)
  • src/api/linker.test.ts (1 hunks)
  • src/api/linker.ts (1 hunks)
  • src/api/opt-in.js (0 hunks)
  • src/api/opt-out.js (0 hunks)
  • src/api/pageview.js (0 hunks)
  • src/api/pageview.test.ts (1 hunks)
  • src/api/pageview.ts (1 hunks)
  • src/api/purchase.js (0 hunks)
  • src/api/purchase.test.ts (1 hunks)
  • src/api/purchase.ts (1 hunks)
  • src/api/query.js (0 hunks)
  • src/api/query.test.ts (1 hunks)
  • src/api/query.ts (1 hunks)
  • src/api/refund.js (0 hunks)
  • src/api/refund.test.ts (1 hunks)
  • src/api/refund.ts (1 hunks)
  • src/api/screenview.js (0 hunks)
  • src/api/screenview.test.ts (1 hunks)
  • src/api/screenview.ts (1 hunks)
  • src/api/set.js (0 hunks)
  • src/api/set.test.ts (1 hunks)
  • src/api/set.ts (1 hunks)
  • src/api/time.js (0 hunks)
  • src/api/time.test.ts (1 hunks)
  • src/api/time.ts (1 hunks)
  • src/attach-api.js (0 hunks)
  • src/bootstrap.js (0 hunks)
  • src/index.js (0 hunks)
  • src/index.test.ts (1 hunks)
  • src/index.ts (1 hunks)
  • src/options.js (0 hunks)
  • src/register-globals.js (0 hunks)
  • src/router.js (0 hunks)
  • src/settings.test.ts (1 hunks)
  • src/settings.ts (1 hunks)
  • src/track-route.test.ts (1 hunks)
  • src/track-route.ts (1 hunks)
  • src/track.js (0 hunks)
  • src/types.ts (1 hunks)
  • src/utils.js (0 hunks)
  • src/utils.test.ts (1 hunks)
  • src/utils.ts (1 hunks)
  • test/add-configuration.spec.js (0 hunks)
  • test/add-routes-tracker.spec.js (0 hunks)
  • test/api/config.spec.js (0 hunks)
  • test/api/custom-map.spec.js (0 hunks)
  • test/api/disable.spec.js (0 hunks)
  • test/api/event.spec.js (0 hunks)
  • test/api/exception.spec.js (0 hunks)
  • test/api/linker.spec.js (0 hunks)
  • test/api/opt-in.spec.js (0 hunks)
  • test/api/opt-out.spec.js (0 hunks)
  • test/api/pageview.spec.js (0 hunks)
  • test/api/purchase.spec.js (0 hunks)
  • test/api/query.spec.js (0 hunks)
  • test/api/refund.spec.js (0 hunks)
  • test/api/screenview.spec.js (0 hunks)
  • test/api/set.spec.js (0 hunks)
  • test/api/time.spec.js (0 hunks)
  • test/basic.spec.js (0 hunks)
  • test/bootstrap.spec.js (0 hunks)
  • test/exports.spec.js (0 hunks)
  • test/options.spec.js (0 hunks)
  • test/register-globals.spec.js (0 hunks)
  • test/track.spec.js (0 hunks)
  • test/utils.spec.js (0 hunks)
  • tsconfig.json (1 hunks)
  • vite.config.js (1 hunks)
  • vue-gtag.d.ts (0 hunks)
💤 Files with no reviewable changes (51)
  • test/api/purchase.spec.js
  • src/api/set.js
  • src/api/query.js
  • test/api/set.spec.js
  • src/api/time.js
  • src/api/refund.js
  • src/api/linker.js
  • src/api/purchase.js
  • src/api/opt-out.js
  • test/api/opt-in.spec.js
  • src/add-routes-tracker.js
  • src/api/opt-in.js
  • src/attach-api.js
  • test/api/pageview.spec.js
  • src/add-configuration.js
  • test/api/refund.spec.js
  • test/api/config.spec.js
  • test/api/linker.spec.js
  • src/api/config.js
  • src/api/event.js
  • test/add-configuration.spec.js
  • test/api/opt-out.spec.js
  • src/router.js
  • src/api/custom-map.js
  • test/api/custom-map.spec.js
  • src/api/exception.js
  • src/register-globals.js
  • test/api/exception.spec.js
  • src/bootstrap.js
  • test/bootstrap.spec.js
  • test/basic.spec.js
  • src/api/disable.js
  • test/api/time.spec.js
  • test/exports.spec.js
  • src/api/screenview.js
  • test/register-globals.spec.js
  • test/track.spec.js
  • src/api/pageview.js
  • test/api/screenview.spec.js
  • test/utils.spec.js
  • test/options.spec.js
  • vue-gtag.d.ts
  • src/api/index.js
  • test/api/event.spec.js
  • src/options.js
  • test/api/query.spec.js
  • test/api/disable.spec.js
  • src/utils.js
  • test/add-routes-tracker.spec.js
  • src/track.js
  • src/index.js
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/deployment.yml

[error] 28-28: trailing spaces

(trailing-spaces)

🔇 Additional comments (106)
.vscode/settings.json (1)

7-10: Great formatting standardization!

These new editor settings ensure consistent code formatting across the project. Setting a tab size of 2 spaces, using spaces instead of tabs, and disabling automatic indentation detection will help maintain uniform code style for all contributors.

tsconfig.json (1)

1-45:

❓ Verification inconclusive

Well-structured TypeScript configuration with one potential conflict

The TypeScript configuration is comprehensive and follows best practices with strict type checking, proper module resolution, and path aliases.

However, there appears to be a contradiction between noEmit: true (line 14), which prevents TypeScript from generating output files, and the declaration settings (lines 33-35) that specify generating declaration files. This might cause unexpected behavior during builds.

Consider clarifying how these settings should work together. If you intend to generate declaration files during the build process, you might need to:

  • Remove noEmit: true for production builds
  • Use conditional tsconfig settings between development and production
  • Or rely on the vite-plugin-dts (as configured in vite.config.js) to handle type generation separately

🏁 Script executed:

#!/bin/bash
# Check if there are any npm scripts that might modify the tsconfig settings during build
grep -r "tsc" package.json

Length of output: 102


Clarify Declaration Generation vs. NoEmit Usage

The TypeScript configuration is well-structured and enforces strict type safety. However, there's an intentional setup where the configuration enables declaration file generation (via "declaration": true, "declarationDir", and "outDir"), yet the "noEmit": true option prevents TypeScript from producing any output. The package.json scripts (using tsc --noEmit) confirm that type checking is performed without actual file emission—indicating that declaration generation is likely managed by a tool like vite-plugin-dts (configured in vite.config.js).

Action items:

  • Verify Intent: Confirm that the current strategy—to rely on vite-plugin-dts (or a similar tool) for generating declaration files instead of TypeScript’s own emission—is intentional.
  • Document Rationale: If this design is intended, add a comment in the tsconfig.json or in your project documentation explaining why declarations are configured yet TypeScript is prevented from emitting files. This will help future maintainers understand the separation of type-checking and declaration generation.
  • Consider Conditional Configurations: Alternatively, if you expect tsc to generate declarations during production builds, consider using conditional tsconfig settings or removing "noEmit": true in the production configuration.
vite.config.js (6)

3-3: Good addition of TypeScript declaration file generation

Adding the vite-plugin-dts plugin is appropriate for a TypeScript project to ensure proper type definitions are generated for consumers.


19-19: Correct Vue global assignment

Changing the Vue global from lowercase to capitalized (Vue instead of vue) is correct as this matches Vue's actual global object name.


23-23: Good choice of minifier

Using terser for minification is a solid choice for optimizing the build output while maintaining compatibility.


25-30: Proper TypeScript declaration configuration

The DTS plugin is configured well to generate type declarations and make them accessible through the package's entry point.


34-37: Enhanced test configuration

Adding mock reset and coverage reporting improves test isolation and provides better visibility into test coverage, which is valuable for maintaining code quality.


9-12:

❓ Verification inconclusive

Moving to TypeScript and dropping older module formats

The switch from JavaScript to TypeScript for the entry point aligns with the project's migration to TypeScript. However, note that reducing support to only ES modules (dropping UMD and CommonJS) might affect compatibility with some environments. Ensure this is an intentional breaking change aligned with your version 3 strategy.


🏁 Script executed:

#!/bin/bash
# Check if there are any references to UMD or CJS usage in docs or issues
grep -r "umd\|cjs\|commonjs" --include="*.md" --include="*.js" .

Length of output: 66


Subject: Verify ES Module-Only Config Change for V3 Migration

The updated configuration switches the entry file to TypeScript and now supports only ES modules. While the grep check did not reveal references to UMD/CommonJS in markdown or JavaScript files, this output is inconclusive. Please manually verify that no documentation, issues, or external dependencies rely on legacy module formats, ensuring that this intentional breaking change fully aligns with your version 3 strategy.

  • Location: vite.config.js, lines 9–12
  • Key Change: Dropped UMD/CommonJS support—only ES modules are provided.
  • Action: Confirm through manual review or further investigation that no parts of the project or its consumers depend on UMD or CommonJS builds.
src/api/time.ts (1)

1-5: Clean implementation of timing event tracking

This function follows good practices by being focused, simple, and well-typed. The Gtag.EventParams type annotation ensures type safety for consumers of this API.

src/api/purchase.ts (1)

1-5: Clean implementation of the purchase tracking function.

The function properly wraps the event function to track purchase events with Google Analytics, following a consistent pattern established in the API design.

src/api/exception.ts (2)

3-8: Well-defined type union for exception parameters.

Good use of a union type to provide flexibility in how exception parameters can be specified, allowing either a simple object with optional properties or the full Gtag.CustomParams.


10-12: Clean implementation of the exception tracking function.

The function correctly wraps the event function to track exception events with Google Analytics.

src/api/set.ts (2)

3-3: Good use of type alias for better code readability.

Using a type alias for the set command parameters improves code readability and maintainability.


5-7: Clean implementation of the set function using rest parameters.

The function correctly uses rest parameters to accept variable arguments and passes them to the query function with the "set" command.

src/api/custom-map.ts (2)

3-3: Appropriate type alias for custom map parameters.

The type alias helps with code readability while maintaining the connection to the underlying Gtag types.


5-9: Clean implementation of the custom map configuration.

The function properly wraps the parameters in an object with the custom_map property before passing to the config function.

.github/workflows/deployment.yml (1)

28-30: Great addition of type checking to the CI workflow.

Adding a typecheck step to the CI workflow is a valuable improvement that will help catch type-related issues early in the development process. This aligns well with the TypeScript implementations mentioned in the PR summary.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 28-28: trailing spaces

(trailing-spaces)

src/api/exception.test.ts (1)

1-18: Well-structured unit test for the exception function.

The test properly verifies that the exception function forwards the correct parameters to the query function with the "exception" event type. Good use of mocking to isolate the test from dependencies.

src/api/set.test.ts (1)

1-14: Good unit test structure for the set function.

The test correctly verifies that the set function forwards parameters to the query function. The mocking approach effectively isolates the test from external dependencies.

src/api/time.test.ts (1)

1-16: Well-implemented unit test for the time function.

This test effectively verifies that the time function correctly triggers the "timing_complete" event with the provided parameters. Good use of mocking to isolate the test functionality.

src/api/linker.test.ts (1)

1-16: Well-structured test with good isolation

This test properly verifies that the linker function correctly passes parameters to the query function. The test structure follows good practices by:

  • Mocking dependencies for isolation
  • Using clear and descriptive test names
  • Verifying the exact arguments passed to the mocked function
src/api/refund.ts (2)

17-19: Function implementation looks good

The implementation correctly calls the event function with the appropriate parameters.


15-15:

❓ Verification inconclusive

Verify Gtag type availability

The code references Gtag.CustomParams without an explicit import. Ensure this type is globally available or properly imported.


🏁 Script executed:

#!/bin/bash
# Check if Gtag type is defined somewhere in the project
echo "Checking for Gtag type definition..."
rg -l "type\s+Gtag|interface\s+Gtag|namespace\s+Gtag|declare\s+namespace\s+Gtag" --type ts

Length of output: 175


Attention: Verify Gtag Global Definition

The code still references Gtag.CustomParams without an explicit import. Our search through the project's TypeScript files did not reveal any in-repository definition (e.g., via a type, interface, or namespace declaration). This suggests you may be relying on a globally provided type—perhaps from an external package such as @types/gtag.js—or an ambient declaration.

  • Action Required: Please verify that the global type for Gtag is indeed available in your project (for example, via an installation of the appropriate type definitions). If not, add the necessary import and/or declare the type explicitly to prevent potential type errors.
src/api/linker.ts (2)

3-8: Well-defined type with appropriate optionality

The LinkerParams type correctly defines which fields are required vs optional. The use of a string union type for url_position is a good practice for restricting values to valid options.


10-12: Function implementation is concise and effective

The linker function has a clear purpose and delegates to the query function appropriately.

src/api/refund.test.ts (2)

1-5:

❓ Verification inconclusive

Check import alignment with implementation

The test imports query directly while the implementation in refund.ts uses the event function. This suggests there might be a mismatch in how the mocking is set up.


🏁 Script executed:

#!/bin/bash
# Check how event.ts is implemented
echo "Checking event.ts implementation..."
cat src/api/event.ts || echo "File not found"

Length of output: 614


Mismatch in Mocking Setup for Refund Tests

The implementation in src/api/event.ts confirms that the event function is used to trigger query calls. However, in src/api/refund.test.ts, the test currently imports and mocks query directly. Since refund.ts (as indicated in the review comment) uses the event function rather than calling query directly, please verify that the mocking in the test aligns with the actual call paths. Adjust the mocks (or imports) accordingly to cover the real implementation flow.


6-16:

❓ Verification inconclusive

Ensure assertion matches implementation

The test expects query to be called with ("event", "refund", ...), but the implementation calls event("refund", ...). This discrepancy should be resolved by either:

  1. Updating the test to match the implementation
  2. Verifying that the event function internally calls query as expected

🏁 Script executed:

#!/bin/bash
# Check how the event function is implemented
echo "Checking how event function calls query..."
rg -A 5 "function event\(" --type ts src/api

Length of output: 489


Mismatch between test assertion and implementation of refund event

The test is asserting that query is called with ("event", "refund", { transaction_id: "transaction_id_value" }), yet the refund function calls event("refund", { transaction_id: "transaction_id_value" }). The current implementation of event in src/api/event.ts (as seen from the snippet) does not clearly show an invocation of query with those parameters.

Please consider one of the following actions:

  • Update the test: Change the expectation to verify that event is called with the appropriate arguments, if that is the intended behavior.
  • Enhance the implementation: Ensure that the event function internally calls query with the correct parameters if the test’s expectation is correct.

Review the implementations in both src/api/refund.test.ts (lines 6–16) and src/api/event.ts to align the behavior and the tests accordingly.

src/add-router-tracking.ts (1)

1-20: Clean implementation of router tracking!

The addRouterTracking function is well-structured and follows the single responsibility principle. It correctly:

  1. Retrieves settings and checks for router existence
  2. Waits for the router to be ready with await router.isReady()
  3. Tracks the initial route
  4. Sets up an event listener for subsequent route changes

The early return pattern when router is absent is a good practice.

src/utils.test.ts (1)

4-47: Well-structured test suite for injectScript

The test coverage is comprehensive, testing:

  1. Basic script tag creation
  2. Script with defer attribute
  3. Link element for domain preconnect

Good job using afterEach to clean up the DOM between tests.

src/api/event.ts (2)

4-5: Type definitions for event parameters look good

The TypeScript type definitions correctly leverage the Gtag types for proper type safety.


7-17: Well-implemented event handling function

The function correctly:

  1. Retrieves settings
  2. Handles the variable arguments pattern
  3. Applies conditional logic for the send_to parameter
  4. Calls the underlying query function

The conditional logic on line 12 ensures that send_to is only set when there are additional accounts and when it hasn't been explicitly defined by the caller.

src/api/custom-map.test.ts (1)

7-23: Well-structured test with good isolation practices

The test correctly verifies the customMap function behavior and follows good practices:

  1. Uses beforeEach to reset settings between tests
  2. Clearly sets up the test state before executing the function
  3. Makes specific assertions about the expected behavior

This approach ensures good test isolation and maintainability.

biome.json (2)

11-11: Well-structured file inclusion for TypeScript.

The updated include pattern now specifically targets TypeScript files, the TypeScript configuration file, and global type declarations, which is more precise than including all files in the src directory.


15-16: Good formatting consistency.

Adding the indentWidth property ensures consistent indentation (2 spaces) across the codebase, which is a standard practice in many JavaScript/TypeScript projects.

src/api/config.ts (2)

5-5: Good type definition using Gtag types.

Using TypeScript's ability to extract parts of existing types via Tail<Gtag.GtagCommands["config"]> is an excellent way to maintain type safety while reusing the Google Analytics type definitions.


7-23: Well-structured implementation with proper error handling.

The function correctly:

  • Uses TypeScript rest parameters with proper typing
  • Gets settings and handles the case when tagId is not available
  • Processes the main tag configuration
  • Handles additional accounts when present

The early returns pattern helps maintain flat code structure and readability.

src/types.ts (3)

4-7: Good type re-exporting practice.

Re-exporting types from vue-router with the alias Route instead of the more verbose RouteLocationNormalizedGeneric improves code readability throughout the codebase.


9-9: Excellent utility type implementation.

The Tail<T> type using conditional types and inference is a great example of advanced TypeScript usage to extract all elements except the first from a tuple type.


11-11: Clear union type for tracking parameters.

Creating a union type PageTrackerParams helps unify the handling of different tracking event types, making the API more consistent and easier to use.

src/api/pageview.test.ts (4)

6-6: Proper test isolation with mocking.

Mocking the query function ensures tests focus on the pageview functionality without side effects from external dependencies.


11-25: Well-structured test setup.

The test setup creates a realistic router configuration with base path, routes, and navigation. Mocking the router's isReady method ensures tests don't depend on actual router initialization.


27-113: Comprehensive test coverage for pageview functionality.

These tests thoroughly cover:

  • Different ways to call the pageview function (path string, route object)
  • Various configuration options (useRouteFullPath)
  • Default parameters (page_location, send_page_view)
  • Title tracking and custom parameters

This extensive test suite ensures the pageview functionality works correctly in all scenarios.


115-147: Good testing of pageTracker integration.

Testing the pageTracker configuration separately in its own describe block is a clean approach. The tests verify both standard path tracking and router base path integration.

src/api/query.ts (3)

1-3: LGTM on the imports.

The imports correctly pull in the necessary dependencies for the module.


4-4: Good use of type inference from Gtag.Gtag.

Using Parameters<Gtag.Gtag> is a clean approach to derive the parameter types from the gtag function signature.


13-21:

❓ Verification inconclusive

Verify gtag integration pattern.

The implementation follows standard gtag.js integration patterns. However, consider the following:

  1. Add type safety for window[dataLayerName] to ensure it's an array
  2. Add error handling for edge cases where window properties aren't set correctly

🏁 Script executed:

#!/bin/bash
# Check how other popular GA libraries type the data layer
rg -A 3 "dataLayer\s*=" --glob="*.ts"

Length of output: 510


Attention: gtag Integration Pattern Review

The implementation follows the standard gtag.js integration approach. However, please consider the following improvements:

  • Type Safety: Ensure that window[dataLayerName] is explicitly typed as an array (e.g., using a check like Array.isArray(window[dataLayerName]) before assigning a default of []).
  • Error Handling: Add safeguards to handle edge cases where the window properties (such as dataLayerName or gtagName) might not be set correctly.

The output from searching the test files shows that other parts of the code (as seen in tests like in src/add-gtag.test.ts) use the string "dataLayer" without additional type validation. This suggests that the current pattern is standard; however, the proposed enhancements would improve robustness.

src/index.ts (4)

1-5: LGTM on the imports.

The imports correctly pull in the necessary dependencies for the module.


15-16: Good use of TypeScript utility types.

The use of Partial<Settings> combined with Required<Pick<Settings, "tagId">> creates a clear type that requires tagId while making other settings optional.


18-25: LGTM on the Vue plugin implementation.

The implementation follows the Vue plugin pattern correctly by:

  1. Setting up the necessary configuration
  2. Returning a function that modifies the Vue app instance
  3. Adding the API to Vue's global properties

27-27: Good export pattern.

Re-exporting from the API module is a clean way to provide a unified interface.

src/add-configuration.test.ts (3)

1-9: LGTM on the test setup.

The test imports, mocking, and date fixing are properly set up. Using mockdate ensures consistent test results regardless of when they run.


10-24: LGTM on the basic configuration test.

This test correctly verifies that:

  1. The JS command is called with the expected date
  2. The config command is called with the correct tagId and default settings

26-58: LGTM on the multiple domains test.

This test thoroughly verifies the correct behavior with multiple accounts, ensuring:

  1. The main tag is configured first without a group
  2. Additional accounts are configured with appropriate settings
  3. Different configurations are correctly merged with defaults
src/add-configuration.ts (5)

1-4: LGTM on the imports.

The imports correctly pull in the necessary dependencies for the module.


5-10: Good implementation of default configuration merging.

The mergeDefaults function ensures consistent default behavior while preserving custom configurations. Setting send_page_view: false as the default aligns with best practices for GTM/GA implementations.


12-24: LGTM on function setup and early return.

The function correctly:

  1. Extracts settings with appropriate destructuring
  2. Returns early if no tagId is provided, preventing errors

25-31: LGTM on linker and main tag configuration.

The implementation correctly:

  1. Sets up the linker if provided in settings
  2. Initializes the GTM library with the current date
  3. Configures the main tag with merged default settings

32-46: LGTM on additional accounts configuration.

The implementation correctly:

  1. Returns early if no additional accounts are provided
  2. Iterates through each account
  3. Configures each with the appropriate tag ID
  4. Merges default settings with account-specific settings and group name
src/add-gtag.ts (1)

6-38: Well-implemented Google Tag Manager integration function

The addGtag function follows a clear and logical flow:

  1. Retrieves settings
  2. Guards against missing tagId
  3. Sets up configuration
  4. Conditionally adds router tracking
  5. Handles script injection with proper error handling

The use of optional chaining for callbacks and the async/await pattern makes the code clean and maintainable.

src/index.test.ts (1)

1-37: LGTM: Concise and focused plugin installation test

The test properly verifies that:

  1. The plugin is installed correctly on a Vue app
  2. The addGtag function is called during installation
  3. The $gtag global property is added with all expected methods

The snapshot test is a good way to ensure all expected methods are present.

src/api/screenview.test.ts (1)

1-71: Comprehensive tests for screenview functionality

The tests cover all important scenarios for the screenview functionality:

  1. Tracking with a string parameter
  2. Including app_name when configured
  3. Using route name from a route object
  4. Falling back to path when route has no name

The test setup is clean and the assertions verify the correct parameters are being passed.

src/api/screenview.ts (3)

1-4: Imports look appropriate

The imports correctly bring in the necessary dependencies for the screenview functionality.


5-10: Well-defined TypeScript types

The type definitions are clear and appropriate. Using optional properties for app_name and screen_name in the Screenview type and creating a union type for ScreenviewParams follows TypeScript best practices.


29-30: Query implementation looks correct

The function correctly calls query with the event type "screen_view" and the constructed template.

src/api/event.test.ts (5)

1-6: Test setup is correctly implemented

The imports and mock setup provide the necessary foundation for testing the event function.


7-14: Basic event functionality test

This test correctly verifies that the event function passes parameters to the query function.


16-27: Default group name test

Comprehensive test verifying that when additionalAccounts are present, the default group name is added.


29-41: Custom group name test

Well-structured test that verifies custom group names are properly used when specified in settings.


43-58: Send_to property preservation test

Good test ensuring that an explicitly provided send_to property takes precedence over the default behavior.

src/track-route.ts (5)

1-5: Imports are appropriate

The file correctly imports the necessary tracking functions, settings, and type definitions.


18-26: Early return pattern is well implemented

The function correctly gets settings and checks for exclusion before proceeding with tracking.


27-34: Template initialization is well-handled

The code correctly handles both function and object templates.


36-46: Conditional tracking based on settings

The function properly differentiates between screenview and pageview tracking based on settings.


48-49: Lifecycle hook for tracking completion

The function properly calls the optional onAfterTrack callback after tracking is complete.

src/utils.ts (4)

1-3: Server detection function is implemented correctly

The isServer function correctly checks for the absence of browser-specific globals.


5-16: Well-structured script injection function signature

The function signature with typed parameters and options is clear and follows TypeScript best practices.


12-25: Early return pattern and script configuration

The function correctly handles server-side execution and configures the script element with the appropriate attributes.


27-34: Preconnect optimization is a good performance enhancement

Adding preconnect functionality can improve load performance for external scripts.

src/api/index.ts (1)

1-12: Well-structured API export pattern

This index file provides a clear, centralized export point for all API modules. The pattern of named exports with default imports is consistent and follows modern JavaScript/TypeScript practices.

src/add-gtag.test.ts (9)

8-12: Good test setup with appropriate mocks

The test file properly mocks all dependencies needed for isolated testing of the addGtag function.


13-15: Good test organization with proper setup

Using beforeEach(resetSettings) ensures each test starts with a clean state, which is a good testing practice.


16-33: Well-structured test for default gtag.js library download

This test thoroughly verifies that the correct URL is constructed and that injectScript is called with the appropriate parameters.


35-53: Good test coverage for custom resource URL

This test correctly verifies the behavior when a custom resource URL is provided, ensuring flexibility in the implementation.


55-66: Callback verification is well-tested

This test correctly verifies that the onReady callback is executed when the plugin is ready.


68-81: Error handling test is thorough

The test properly mocks a rejection in injectScript and verifies that the error callback is triggered, which is essential for robust error handling.


83-91: Initial configuration test is concise and clear

This test verifies that addConfiguration is called, ensuring proper initialization.


93-110: Router tracking test is comprehensive

This test creates a real router instance and verifies that both addRouterTracking and addConfiguration are called when router settings are provided.


112-116: Edge case handling is well-tested

This test verifies that addConfiguration is not called when the tagId is missing, ensuring the function handles this edge case appropriately.

package.json (3)

39-45: Good package exports configuration

The exports configuration follows modern package.json practices and clearly defines the entry points for the package.


46-48: Simplified files array

The files array has been simplified to only include the "dist" directory, which is cleaner and more maintainable.


58-66: Good TypeScript dependencies added

The addition of TypeScript-related dependencies like @types/gtag.js, @types/node, and vite-plugin-dts supports the transition to TypeScript.

src/api/pageview.ts (3)

5-12: Clear type definitions that enhance code maintainability

The type definitions for Pageview, PageviewParams, and PageviewOptions are well-structured and provide good type safety for the function parameters.


14-19: Well-implemented path normalization helper

The getPathWithBase function correctly handles the normalization of base and path components, ensuring proper path construction regardless of trailing/leading slashes.


47-53: Good default value handling

The function correctly checks for undefined values rather than falsy values, which is a good TypeScript practice and prevents unexpected behavior with valid falsy values.

src/track-route.test.ts (5)

1-7: Well organized test imports and mocking setup

The imports are well organized, and the API query module is properly mocked. This ensures isolation of the test from actual network calls.


8-34: Good test suite setup with proper router configuration

The test setup creates a clean environment for each test by resetting settings and creating a new router with two routes. The router's isReady method is properly mocked to avoid waiting for actual router initialization.


35-53: Comprehensive screenview tracking test

The test properly validates that screenview tracking works as expected, with all expected parameters being passed to the query function.


101-135: Thorough callback testing

The tests correctly verify that both before and after tracking callbacks are invoked with the current route.


137-179: Comprehensive template testing

Tests for both static templates and function-based templates are properly implemented and verify that the correct parameters are passed to the query function.

src/settings.ts (7)

1-3: Good type imports

Importing specific types from other modules shows good modular design and type safety practices.


4-9: Well-defined ConfigParams type

The ConfigParams union type provides a clear interface for configuration parameters that can be passed to Gtag.


10-13: Flexible PageTrackerTemplate type

The PageTrackerTemplate type allows for both static and dynamic template definitions, providing good flexibility for developers.


14-22: Comprehensive PageTracker type definition

The PageTracker type includes all necessary configurations for route tracking, including exclusion options and callbacks.


24-38: Well-structured Settings type

The Settings type provides a comprehensive configuration interface with sensible optional and required properties.


40-47: Properly defined defaultSettings

The defaultSettings object is correctly marked as Readonly to prevent accidental modifications, and it provides sensible defaults for required properties.


51-55: Simple and effective settings access functions

The getSettings and resetSettings functions provide a clean API for accessing and resetting settings.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (6)
src/api/ecommerce.ts (1)

1-12: Looks good, but JSDoc could be more comprehensive.

The implementation is clean and follows good TypeScript practices with proper typing. The function correctly delegates to the generic event function.

However, the JSDoc comment specifically references "refund" events while the function handles all e-commerce events. Consider expanding the documentation to cover the broader scope of e-commerce events.

 /**
- * @see https://developers.google.com/analytics/devguides/collection/ga4/reference/events?client_type=gtag#refund
+ * E-commerce event tracking for GA4
+ * @see https://developers.google.com/analytics/devguides/collection/ga4/reference/events?client_type=gtag#ecommerce
  */
src/api/event.ts (3)

5-5: Consider moving EventParams type to the types module.

The EventParams type is defined inline, which is inconsistent with the pattern used for EventNames that's imported from a dedicated types file. For better code organization and maintainability, consider moving this type definition to the types/event.ts file.


7-15: Function modifies input parameters directly.

The function directly modifies the params object by assigning to params.send_to. This mutation could lead to unexpected behavior for callers if they use the same object elsewhere.

export default function event(name: EventNames, params: EventParams) {
  const { groupName, additionalAccounts } = getSettings();

+  const eventParams = { ...params };
  
  if (params.send_to === undefined && additionalAccounts?.length) {
-    params.send_to = groupName;
+    eventParams.send_to = groupName;
  }

-  query("event", name, params);
+  query("event", name, eventParams);
}

10-12: Conditional check could be more explicit.

The conditional check additionalAccounts?.length works but could be more explicit about the intent to check if the array has elements.

-  if (params.send_to === undefined && additionalAccounts?.length) {
+  if (params.send_to === undefined && additionalAccounts && additionalAccounts.length > 0) {
src/api/ecommerce.test.ts (1)

6-16: Test coverage could be expanded.

The current test covers only the "refund" event type with minimal parameters. Consider adding tests for:

  1. Other e-commerce event types (purchase, view_item, etc.)
  2. More complex parameter scenarios (items array, additional properties)
  3. Edge cases and error handling

This would provide more comprehensive validation of the functionality.

describe("ecommerce", () => {
  it("should use the ecommerce event", () => {
    ecommerce("refund", {
      transaction_id: "transaction_id_value",
    });

    expect(query).toHaveBeenCalledWith("event", "refund", {
      transaction_id: "transaction_id_value",
    });
  });
+
+  it("should handle purchase events with items", () => {
+    ecommerce("purchase", {
+      transaction_id: "transaction_id_value",
+      currency: "USD",
+      value: 123.45,
+      items: [
+        {
+          item_id: "item1",
+          item_name: "Test Item",
+          price: 123.45,
+          quantity: 1
+        }
+      ]
+    });
+
+    expect(query).toHaveBeenCalledWith("event", "purchase", {
+      transaction_id: "transaction_id_value",
+      currency: "USD",
+      value: 123.45,
+      items: [
+        {
+          item_id: "item1",
+          item_name: "Test Item",
+          price: 123.45,
+          quantity: 1
+        }
+      ]
+    });
+  });
});
src/types.ts (1)

13-16: Consider stricter typing for CustomParams

While the any type is sometimes necessary, consider if you could use a more specific type like unknown or a union of expected types to improve type safety while still allowing flexibility.

export type CustomParams = {
  // biome-ignore lint/suspicious/noExplicitAny:
-  [key: string]: any;
+  [key: string]: string | number | boolean | null | undefined | object;
};
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa715ba and 05b0571.

📒 Files selected for processing (10)
  • src/api/ecommerce.test.ts (1 hunks)
  • src/api/ecommerce.ts (1 hunks)
  • src/api/event.ts (1 hunks)
  • src/api/index.ts (1 hunks)
  • src/api/set.test.ts (1 hunks)
  • src/index.test.ts (1 hunks)
  • src/types.ts (1 hunks)
  • src/types/ecommerce.ts (1 hunks)
  • src/types/event.ts (1 hunks)
  • vite.config.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/api/set.test.ts
  • src/index.test.ts
🔇 Additional comments (14)
vite.config.js (6)

3-3: Good addition of the DTS plugin for TypeScript support.

Adding the vite-plugin-dts plugin is appropriate for generating TypeScript declaration files, which is essential for the TypeScript migration mentioned in the PR objectives.


9-9: Correctly updated entry point to TypeScript.

The change from a JavaScript to TypeScript entry point aligns with the TypeScript rewrite mentioned in the PR objectives.


19-19: Standard Vue global name correction.

Changing from lowercase "vue" to capitalized "Vue" follows standard conventions for global variable naming in the Vue ecosystem.


24-29: Good TypeScript integration configuration.

The DTS plugin configuration is appropriate for generating TypeScript declaration files:

  • insertTypesEntry: true properly adds types entry to package.json
  • rollupTypes: true ensures bundling of type definitions

This will help TypeScript users get proper type support when using the library.


33-36: Improved test configuration with coverage reporting.

Adding mockReset: true and coverage reporting with text and HTML outputs enhances the testing framework capabilities and provides better feedback on test coverage.


11-12:

✅ Verification successful

Verify impact of formats reduction to ESM-only.

The formats have been simplified to only include "es" (ESM), removing "umd" and "cjs" formats that may have been present before. While this aligns with the "esm-only build" objective, ensure this change won't break backward compatibility for users relying on UMD or CommonJS.


🏁 Script executed:

#!/bin/bash
# Check if the package.json specifies module type and if there's any documentation about the format change
cat package.json | grep -E '"type"|"main"|"module"'
grep -r "require\(.*vue-gtag" --include="*.js" .

Length of output: 199


Below is an updated verification step to gather the necessary information. It appears the previous grep command failed due to improper escaping of the parentheses. To verify if any CommonJS (i.e. require("vue-gtag") usage still exists in the repo (which would indicate users expecting UMD/CJS support), please run the following shell script:

Once you review the output of this command:

  • If there are no hits, it indicates that there’s no evidence of legacy usage in the codebase, so the removal of UMD/CJS formats should not affect any consumers.
  • If there are matches, then please verify if these consumers need continued support for UMD or CommonJS builds.

After this verification, you can update the review comment accordingly.



🏁 Script executed:

#!/bin/bash
# Search for CommonJS usage of vue-gtag within JavaScript files using ripgrep
rg 'require\("vue-gtag' .

Length of output: 27


ESM-Only Build Confirmed – No Legacy Usage Found

  • The package configuration shows "type": "module" and the main export is set to the ESM file ("./dist/vue-gtag.js").
  • A search for CommonJS usage (require("vue-gtag") did not return any results, indicating there’s no legacy dependency on UMD or CommonJS formats.

Based on these findings, the reduction to an ESM-only build should not affect backward compatibility.

src/types.ts (4)

1-7: Good imports and exports from vue-router

The file correctly imports specific types from internal modules and re-exports types from vue-router with appropriate naming. This provides a centralized type system for the rest of the application.


9-9: Well-designed utility type for tuple manipulation

The Tail<T> type utility is correctly implemented to extract all elements of a tuple except the first one, which is useful for parameter handling in function calls.


11-11: Good union type for page tracking parameters

Defining PageTrackerParams as a union of Pageview | Screenview creates a clean API for page tracking functions that can handle both types of views.


18-18: Good pattern for extensible parameters

The ParamsWithCustom<T> type provides a clean way to allow both strictly typed parameters and custom parameters, which is valuable for a library that needs to be flexible.

src/api/index.ts (1)

1-11: Well-organized API exports

The barrel file pattern used here provides a clean, centralized way to export all API modules. This makes imports more manageable throughout the codebase and follows best practices for module organization.

src/types/ecommerce.ts (3)

1-15: Comprehensive ecommerce event types

The EcommerceEventName type provides a complete set of GA4 ecommerce event names, which ensures type safety when tracking these events. This will help prevent typos and provide autocomplete suggestions.


17-92: Excellent documentation and typing for ecommerce items

The EcommerceItem type is well-documented with comprehensive JSDoc comments that explain each field's purpose. The type definition closely follows GA4 ecommerce parameters specifications, which will help ensure proper tracking implementation.


94-131: Well-structured ecommerce parameters with clear documentation

The EcommerceParams type properly defines all the necessary fields for ecommerce event tracking with detailed JSDoc comments. The cross-references to item-level parameters (with explanations of precedence) are particularly helpful.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (6)
tsconfig.json (1)

29-33: Declaration Files & Output Directories:
Declaration file generation is enabled and configured to output to "./dist". However, since "noEmit": true is specified earlier, TypeScript won’t produce any output by default. Make sure this is intentional—if you intend to generate declaration files, you might need to adjust the build process or add "emitDeclarationOnly": true in a dedicated build configuration.

src/utils.ts (3)

5-15: Improve Promise handling for server-side execution

The early return in the server-side case doesn't properly resolve the Promise.

export async function injectScript(
  url: string,
  options?: {
    preconnectOrigin?: boolean;
    defer?: boolean;
  },
): Promise<void> {
  return new Promise((resolve, reject) => {
    if (isServer()) {
-      return;
+      return resolve();
    }

27-36: Add error handling for URL parsing during preconnect

When creating a preconnect link, the code assumes the URL is valid and can be parsed by the URL constructor, which could throw an error with invalid URLs.

if (options?.preconnectOrigin) {
  const link = document.createElement("link");

+  try {
    const resource = new URL(url);
    
    link.href = resource.origin;
    link.rel = "preconnect";
    
    head.appendChild(link);
+  } catch (error) {
+    console.warn("Failed to parse URL for preconnect:", error);
+  }
}

38-42: Consider adding cleanup functionality

The script elements added to the DOM will remain there permanently. For components or modules that might be mounted and unmounted multiple times, consider adding a way to clean up these elements.

src/settings.ts (2)

47-56: Consider using environment variables for the resource URL

The default Google Tag Manager URL is hardcoded. For greater flexibility, consider making this configurable via environment variables.

const defaultSettings: Readonly<Settings> = {
  resource: {
-    url: "https://www.googletagmanager.com/gtag/js",
+    url: import.meta.env?.VITE_GTAG_URL || "https://www.googletagmanager.com/gtag/js",
    preconnect: false,
    deferred: false,
  },
  // ...
};

58-58: Consider using a more immutable approach for settings management

The use of a mutable variable for settings could lead to unexpected behavior in certain scenarios, especially in concurrent environments.

You might consider a more functional approach that doesn't rely on a mutable global variable, perhaps using a settings manager class or a reactive state system depending on your framework.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 05b0571 and 67fdc1e.

📒 Files selected for processing (12)
  • package.json (4 hunks)
  • src/add-gtag.test.ts (1 hunks)
  • src/add-gtag.ts (1 hunks)
  • src/api/ecommerce.ts (1 hunks)
  • src/api/query.ts (1 hunks)
  • src/index.ts (1 hunks)
  • src/settings.test.ts (1 hunks)
  • src/settings.ts (1 hunks)
  • src/types/event.ts (1 hunks)
  • src/utils.test.ts (1 hunks)
  • src/utils.ts (1 hunks)
  • tsconfig.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
  • src/utils.test.ts
  • src/api/ecommerce.ts
  • src/settings.test.ts
  • src/api/query.ts
  • src/types/event.ts
  • src/add-gtag.ts
  • src/add-gtag.test.ts
  • src/index.ts
  • package.json
🔇 Additional comments (13)
tsconfig.json (7)

1-8: Compiler Options & Target Declaration:
The configuration correctly targets ES2020 and sets the module system to ESNext. Using "useDefineForClassFields": true ensures that class field semantics are aligned with the latest ECMAScript specifications. The specified libraries ("ES2020", "DOM", and "DOM.Iterable") are appropriate for modern web applications.


9-15: Bundler Mode Configuration:
The settings under the "Bundler mode" comment are well defined. "moduleResolution": "bundler", along with enabling TypeScript extensions for imports, JSON module resolution, and isolated modules, supports an ESM-only build effectively. Note that "noEmit": true means TypeScript will only perform type checking without producing output, so verify that this aligns with your build and release strategy (e.g., if a separate tool is used for emitting declarations).


16-21: Strict Type Checking Enforcement:
Enabling "strict", "noUnusedLocals", "noUnusedParameters", and "noFallthroughCasesInSwitch" promotes robust and error-free code. These options are recommended to catch potential issues early during development.


22-25: Vue-Specific JSX Settings:
The "jsx": "preserve" and "jsxImportSource": "vue" settings are correctly configured to support Vue’s JSX transformation. Ensure that your Vue version supports these options or consider updating if needed.


26-29: Path Aliases Configuration:
Setting "baseUrl": "." helps with resolving modules relative to the project root. This simple aliasing works well if your directory structure is straightforward. If needed, you might later extend this with additional aliases for deeper directory structures.


34-38: Additional Checks and Type Definitions:
The options "forceConsistentCasingInFileNames": true and "esModuleInterop": true are standard best practices to avoid common module and casing issues. Including types for "@types/gtag.js", "@types/node", and "vitest/globals" ensures that your project has access to the necessary global type declarations for these libraries.


40-42: File Inclusion and Exclusion Settings:
The "include" and "exclude" settings target the TypeScript source (.ts and .d.ts) and properly exclude "node_modules" and "dist". This configuration helps keep the compilation focused on your source files.

src/utils.ts (3)

1-3: Good server detection implementation

The isServer utility function correctly checks for both window and document to determine server-side rendering, which is more robust than just checking for one of them.


49-51: Good null check and array check in isObject function

The isObject function correctly checks for null, type, and also ensures the value is not an array, which is important for accurate deep merging.


53-73: Robust deep merge implementation

The deepMerge function is well-implemented:

  • Creates a new object rather than mutating the target
  • Uses Object.prototype.hasOwnProperty.call for security
  • Correctly handles nested objects with recursive merging
  • Uses proper TypeScript generics for type safety

This is good for handling nested configuration settings.

src/settings.ts (3)

1-4: Good organization with explicit imports

The imports are clearly organized, importing types from relevant modules and the deepMerge utility from the utils file we just reviewed.


5-45: Well-structured type definitions with appropriate optionality

The type definitions are comprehensive and provide good structure for the configuration:

  • ConfigParams uses the union of Gtag types
  • PageTrackerTemplate allows for both static params and dynamic functions
  • Settings clearly marks which properties are optional vs required

This will help catch configuration errors at compile time.


66-68: Good implementation using deep merge

You've addressed the previous review comment by using the deepMerge utility function from utils.ts, which ensures that nested objects are properly merged.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
src/add-gtag.test.ts (2)

20-37: Consider adding call count assertions.

The test verifies that injectScript is called with the correct URL parameters, which is good. For additional precision, consider adding an assertion for the number of times it was called.

    expect(utils.injectScript).toHaveBeenCalledWith(url, {
      preconnectOrigin: preconnect,
      defer: false,
    });
+   expect(utils.injectScript).toHaveBeenCalledTimes(1);

117-121: Consider additional verification for missing tagId case.

While the test correctly verifies that addConfiguration is not called when the tagId is missing, it would be beneficial to also check that injectScript is not called in this scenario.

  it("should not bootstrap gtag if tagId is missing", async () => {
    await addGtag();

    expect(addConfiguration).not.toHaveBeenCalled();
+   expect(utils.injectScript).not.toHaveBeenCalled();
  });
src/settings.ts (2)

49-58: Consider revisiting default resource values.

The default resource config sets preconnect and deferred to false. If users typically benefit from loading analytics scripts proactively, consider setting them to true or exposing that as a more visible config option. This might improve performance for many use cases.


60-70: Guard against concurrency or SSR usage.

settings is a mutable global variable. If your library is used in server-side rendering or concurrent environments, shared mutable state could lead to requests overriding each other’s configurations. Consider externalizing or scoping these settings per request or instance.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71eeb1c and 130673c.

📒 Files selected for processing (4)
  • package.json (4 hunks)
  • src/add-gtag.test.ts (1 hunks)
  • src/add-gtag.ts (1 hunks)
  • src/settings.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/add-gtag.ts
  • package.json
🔇 Additional comments (12)
src/add-gtag.test.ts (10)

1-16: Imports and mocks setup looks good.

The imports and mocks are well-organized. The module setup properly isolates the testing of addGtag from its dependencies.


17-19: Good test setup with proper isolation.

Using beforeEach(resetSettings) ensures that each test starts with a clean state, which is essential for test isolation.


39-58: Test for custom resource URL is comprehensive.

The test correctly verifies that a custom resource URL is used when specified in the settings.


60-71: Callback test is well-structured.

The test for the onReady callback properly verifies that the callback is fired when the plugin is ready.


73-86: Error handling test is thorough.

Good job on testing the error case by mocking injectScript to reject with an error and verifying that the error callback is called.


88-96: Initial configuration test is clear.

The test correctly verifies that addConfiguration is called after invoking addGtag.


98-115: Router tracking test is comprehensive.

The test properly creates a router instance and verifies that router tracking is initialized when a router is provided in the settings.


123-132: Custom script test is well-implemented.

The test correctly verifies that the script is not injected when useCustomScript is set to true.


134-146: OnReady with custom script test is thorough.

The test correctly verifies that the onReady callback is fired even when using a custom script loader.


1-147: Overall test coverage is excellent.

The test suite comprehensively covers the functionality of the addGtag function, including script loading, configuration, router tracking, error handling, and callback execution. The tests are well-organized and provide good coverage of various scenarios.

src/settings.ts (2)

5-47: Well-structured type definitions.

Your type definitions for ConfigParams, PageTrackerTemplate, PageTracker, Resource, and Settings are clearly laid out. They enhance readability and help ensure type safety across the project. Great job!


1-3:

✅ Verification successful

Confirm presence of Gtag type definitions.

You're referencing Gtag.* types, such as Gtag.ControlParams and Gtag.EventParams. Please ensure they are installed or declared in your type definitions (e.g., via @types/gtag.js) to avoid potential TypeScript errors.

If necessary, you can run a script to confirm whether @types/gtag.js (or comparable definitions) are declared in your package.json:


🏁 Script executed:

#!/bin/bash
rg -A 3 '"@types/gtag' package.json

Length of output: 37


I'm not seeing any references to "@types/gtag" in your package.json, which raises the possibility that the type definitions for Gtag might be declared locally or via another dependency. To gather more clarity on this, please run the following shell script to search the repository for any custom type declarations for Gtag:

Once you review the output, please verify if the necessary Gtag.* type definitions (such as Gtag.ControlParams and Gtag.EventParams) are present. If they’re not found, you may need to install @types/gtag.js or declare them manually to avoid potential TypeScript errors.


🏁 Script executed:

#!/bin/bash
# Search for any declaration of the Gtag namespace within the repository, which would confirm local or custom type definitions.
rg -n "declare namespace Gtag" .

Length of output: 77


Gtag type definitions confirmed

The custom declaration for the Gtag namespace is present in src/gtag.d.ts (line 2). This confirms that Gtag.ControlParams, Gtag.EventParams, and other related types are properly defined. No further changes are needed regarding the inclusion of @types/gtag.js in your package.json.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/settings.ts (1)

32-48: Consider freezing certain properties in Settings if immutability is desired.

While this design works, you could optionally use Readonly types for properties like resource or pageTracker if you want to prevent accidental mutations. That said, the current approach is functional enough if mutability is an intentional choice.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 130673c and 92aaefd.

📒 Files selected for processing (8)
  • src/api/config.ts (1 hunks)
  • src/api/pageview.ts (1 hunks)
  • src/api/screenview.ts (1 hunks)
  • src/settings.ts (1 hunks)
  • src/track-route.ts (1 hunks)
  • src/types/page-tracker.ts (1 hunks)
  • src/types/router.ts (1 hunks)
  • src/types/utils.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/types/page-tracker.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/api/config.ts
  • src/track-route.ts
  • src/api/screenview.ts
🔇 Additional comments (11)
src/settings.ts (5)

1-4: Imports look consistent.

All imported types and the deepMerge utility are utilized properly. Good job keeping imports scoped and clear.


6-30: Type definitions appear comprehensive.

The union types for ConfigParams and PageTrackerTemplate encapsulate their possible shapes, and PageTracker and Resource fields are clearly documented. This structure helps maintain clarity for multiple usage scenarios.


50-60: Default settings are well-defined.

Providing default values for key fields promotes consistency and reduces boilerplate.


61-67: Settings accessors are straightforward and clean.

The getSettings and resetSettings functions are intuitive: they promote strong maintainability by exposing clear read/reset behavior for global settings.


69-71: Usage of deepMerge ensures nested objects are handled correctly.

The decision to leverage a deep merge function addresses the issue of nested object updates that would be lost with a shallow merge.

src/types/utils.ts (1)

1-1: LGTM! Good utility type for tuple manipulation.

The Tail<T> type is a well-implemented utility type that extracts all elements except the first from a tuple type. This pattern is useful when working with function arguments or implementing functional programming concepts in TypeScript.

src/types/router.ts (1)

1-4: LGTM! Clean type exports from vue-router.

This is a clean approach to re-exporting and renaming types from vue-router, which centralizes router type imports and simplifies the type names for use throughout the application.

src/api/pageview.ts (4)

5-7: LGTM! Clear and meaningful type definitions.

These types provide a flexible API for the pageview function, allowing users to pass a simple string path, a Route object, or a full Gtag config object.


9-12: LGTM! Well-defined options interface.

The options interface provides good flexibility for controlling how the page path is constructed.


14-19: LGTM! Path normalization helper function.

This utility function handles path normalization correctly, ensuring that the base and path are properly joined regardless of whether they include trailing/leading slashes.


47-53: LGTM! Good defaults for page tracking.

Setting default values for page_location and send_page_view ensures consistent tracking behavior even when these values aren't explicitly provided.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/api/opt.ts (1)

3-11: Consider adding type safety for window property access

The dynamic property access on the window object could benefit from better TypeScript type safety.

+// Add a type declaration to improve type safety
+declare global {
+  interface Window {
+    [key: `ga-disable-${string}`]: boolean | undefined;
+  }
+}

 export function optOut(tagId?: TagId) {
   const { tagId: settingsTagId } = getSettings();
   window[`ga-disable-${tagId ?? settingsTagId}`] = true;
 }

 export function optIn(tagId?: TagId) {
   const { tagId: settingsTagId } = getSettings();
   window[`ga-disable-${tagId ?? settingsTagId}`] = undefined;
 }
src/api/opt.test.ts (1)

4-43: Consider adding tests for edge cases

While the current tests cover the basic functionality well, consider adding tests for:

  1. Invalid tag IDs
  2. Behavior when settings are missing or incomplete
  3. Multiple consecutive calls to optIn/optOut
it("should handle missing settings gracefully", () => {
  // Test with empty or undefined settings
  updateSettings({});
  optOut();
  // Verify behavior
});

it("should handle consecutive calls", () => {
  updateSettings({ tagId: "UA-1" });
  optOut();
  optOut(); // Call again
  expect(window["ga-disable-UA-1"]).toBe(true);
  
  optIn();
  optIn(); // Call again
  expect(window["ga-disable-UA-1"]).toBeUndefined();
});
src/api/index.ts (1)

12-12: Different export style for opt module

While other modules use default exports, the opt module uses named exports. This is not necessarily a problem, but it creates an inconsistency in the API design.

Consider either:

  1. Changing opt.ts to use a default export for consistency, or
  2. Documenting why this module uses a different export style
- export { optIn, optOut } from "./opt";
+ export { default as opt } from "./opt";

This would require changing opt.ts to:

export default {
  optIn,
  optOut
};
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 92aaefd and 5e9452a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • src/api/index.ts (1 hunks)
  • src/api/opt.test.ts (1 hunks)
  • src/api/opt.ts (1 hunks)
  • src/index.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/index.test.ts
🔇 Additional comments (6)
src/api/opt.ts (2)

1-6: Implement optOut function correctly to disable Google Analytics tracking

The implementation follows Google's standard approach for disabling tracking via the window["ga-disable-{tag}"] property. The function properly handles both explicit tag IDs and fallback to settings.


8-11: Implement optIn function correctly to re-enable tracking

The implementation correctly sets the window property to undefined to re-enable tracking. This approach follows Google Analytics conventions for opting users back into tracking.

src/api/opt.test.ts (3)

4-17: Tests for optOut functionality look good

The test properly validates that when optOut() is called, the appropriate window property is set to true, correctly disabling tracking for the configured tag ID.


19-28: Tests for optIn functionality look good

The test correctly verifies that after opting out and then opting in, the tracking property is reset to undefined, re-enabling tracking.


30-43: Tests for custom tagId functionality look good

The nested test suite properly verifies the behavior with explicit tagId parameters for both optOut and optIn functions.

src/api/index.ts (1)

1-11: Well-organized exports for API modules

The index file provides a clean, centralized point of access for all API functionality, making imports easier in consuming code.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/api/opt.ts (1)

6-6: Consider adding SSR safety check.

Direct window access could cause issues during server-side rendering if window is undefined.

- window[`ga-disable-${tagId ?? settingsTagId}`] = value;
+ if (typeof window !== 'undefined') {
+   window[`ga-disable-${tagId ?? settingsTagId}`] = value;
+ }
src/api/opt.test.ts (1)

47-73: Consider adding a test for mixed account handling.

Add a test case where additionalAccounts are configured but a specific tagId is provided to the opt functions, verifying that only the specified account is affected.

it("should only opt out the specified account when tagId is provided", () => {
  optOut("UA-1");
  
  expect(window["ga-disable-UA-1"]).toBe(true);
  expect(window["ga-disable-UA-2"]).toBeUndefined();
});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e9452a and 0be917b.

📒 Files selected for processing (2)
  • src/api/opt.test.ts (1 hunks)
  • src/api/opt.ts (1 hunks)
🔇 Additional comments (8)
src/api/opt.ts (4)

3-15: Well-structured helper function for property assignment.

The assignProperty function appropriately handles both single tagId and additional accounts scenarios.


8-10: Verify the early return condition.

The function returns early if there are no additional accounts OR a specific tagId was provided. Make sure this is the intended behavior.


17-25: Well-documented optOut implementation.

The function correctly implements Google Analytics opt-out mechanism by setting the ga-disable-{tagId} property to true.


27-35: Correct implementation of optIn functionality.

The function properly uses undefined rather than false to enable tracking, which aligns with Google Analytics documentation.

src/api/opt.test.ts (4)

4-9: Good test setup with proper state reset.

The beforeEach hook correctly resets settings and window properties before each test, preventing test pollution.


11-30: Comprehensive tests for basic opt functionality.

Tests cover both optOut and optIn behavior with the default tagId from settings.


32-45: Good testing of custom tagId scenario.

Tests verify that opt functions work correctly when a specific tagId is provided.


47-73: Well-structured tests for additional accounts.

Tests confirm that all accounts are properly opted in/out when using additional accounts configuration.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/add-router.tracking.test.ts (2)

1-1: Consider filename consistency.

The filename add-router.tracking.test.ts uses a dot, while the imported module is add-router-tracking. Consider standardizing to either hyphen-based or dot-based naming for consistency across the codebase.


13-130: Consider adding tests for edge cases.

The current tests cover the main scenarios well, but consider adding tests for these edge cases:

  1. When pageTracker.router is not set (which should trigger an early return in addRouterTracking)
  2. Error handling if router.isReady() rejects
it("should not track when router is not set", async () => {
  updateSettings({ pageTracker: {} });
  
  await addRouterTracking();
  
  expect(trackRoute).not.toHaveBeenCalled();
});

it("should handle router ready rejection", async () => {
  const router = createRouter({
    history: createWebHistory(),
    routes,
  });
  
  vi.spyOn(router, "isReady").mockRejectedValue(new Error("Router not ready"));
  
  updateSettings({ pageTracker: { router } });
  
  await expect(addRouterTracking()).rejects.toThrow("Router not ready");
  expect(trackRoute).not.toHaveBeenCalled();
});
src/types/settings.ts (1)

15-32: Fix typos in JSDoc comments

There are two typos in the comment for skipSamePath:

-  /** Avoid traking route changes triggerd by the same path */
+  /** Avoid tracking route changes triggered by the same path */
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b3be8a0 and 4ce1670.

📒 Files selected for processing (8)
  • package.json (4 hunks)
  • src/add-configuration.ts (1 hunks)
  • src/add-router-tracking.ts (1 hunks)
  • src/add-router.tracking.test.ts (1 hunks)
  • src/api/opt.ts (1 hunks)
  • src/index.ts (1 hunks)
  • src/settings.ts (1 hunks)
  • src/types/settings.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/api/opt.ts
  • src/add-configuration.ts
  • src/add-router-tracking.ts
  • src/index.ts
  • package.json
🔇 Additional comments (13)
src/add-router.tracking.test.ts (3)

1-131: Well-structured test suite for router tracking functionality.

The tests comprehensively cover the main router tracking scenarios and demonstrate good testing practices with proper isolation and mocking. The implementation aligns well with the PR objective of rewriting the codebase in TypeScript.


57-92: Good coverage of identical paths tracking.

This test correctly verifies that identical paths are tracked by default, which ensures that the skipSamePath option works as intended when enabled. The test assertions are precise and thorough.


94-129: Thorough testing of the skipSamePath option.

The test properly verifies that when skipSamePath is enabled, identical consecutive paths are not tracked multiple times. This is an important configuration option that impacts user analytics data quality.

src/types/settings.ts (5)

1-4: Good choice of type imports for settings

The imports are clean and specific, pulling in only the necessary types from other modules.


5-9: Clean type definition for ConfigParams

The union type effectively aggregates the various Gtag parameter types, making the configuration API more flexible.


11-13: Well-designed PageTrackerTemplate type

This type allows for both static parameter objects and dynamic functions that generate parameters based on routes, providing flexibility in how tracking is configured.


34-41: Well-structured Resource type with clear defaults

The Resource type is cleanly defined with JSDoc comments indicating default values, which helps developers understand the configuration options.


43-70: Comprehensive Settings type with good documentation

The Settings type is well-structured with thorough JSDoc comments. The optional/required properties are appropriately marked, allowing for flexible configuration while ensuring essential properties are provided.

src/settings.ts (5)

1-3: Good imports with specific type references

The imports are clean and specific, pulling in only what's needed for the module.


4-13: Well-defined default settings with sensible values

The default settings are defined as a readonly object, which is a good practice to prevent accidental modification.


15-15: Settings initialization looks good

Properly initializes the settings with the default values.


17-25: Excellent implementation of deepMerge in updateSettings

The implementation now uses deepMerge instead of a shallow merge, which correctly handles nested object properties as suggested in the previous review.


15-25:

❓ Verification inconclusive

Consider implications of mutable global state

The current implementation uses a mutable global settings variable. While common for configuration management, this approach can create challenges for testing and potential race conditions in concurrent scenarios.

Consider alternatives:

  1. A more immutable approach returning new settings objects
  2. A configuration context pattern if using React
  3. At minimum, ensuring thread safety for concurrent access

🏁 Script executed:

#!/bin/bash
# Check for concurrent access to settings in the codebase

# Look for direct assignments to settings variable
echo "Checking for direct assignments to settings variable..."
rg "settings\s*=" --glob "*.ts" --glob "*.js" -A 1 -B 1

# Look for potential concurrent access patterns
echo "Checking for potential concurrent access patterns..."
rg "updateSettings" --glob "*.ts" --glob "*.js" -A 3 -B 3

# Look for async operations that might access settings
echo "Checking for async operations that might access settings..."
rg "(setTimeout|setInterval|Promise).+getSettings" --glob "*.ts" --glob "*.js"

Length of output: 16586


Action Required: Revisit Global Settings Mutation Pattern

The current design uses a mutable global variable for settings (with direct assignments in both resetSettings and updateSettings). Our verification across the codebase confirms that this pattern is consistently used (as seen in multiple test files) and no asynchronous access to getSettings was detected. However, the mutable state design still bears potential risks:

  • While the synchronous usage today keeps race conditions at bay, future asynchronous or concurrent operations might expose thread-safety issues.
  • Consider refactoring to return new settings objects (an immutable approach) to mitigate these risks.
  • If using a UI framework like React, adopting a context-based pattern could provide a more robust configuration management solution.

Please verify that current usage aligns with the expected application context and address potential concurrency concerns if the operational profile changes in the future.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (3)
src/types/settings.ts (2)

24-25: Fix typo in JSDoc comment.

There's a typo in the comment: "traking" should be "tracking".

-  /** Avoid traking route changes triggerd by the same path */
+  /** Avoid tracking route changes triggered by the same path */

57-58: Consider making resource optional with internal defaults.

The resource property is currently required, but since your JSDoc comments indicate default values for its properties, you might want to make the entire resource property optional and handle defaults internally.

-  /** gtag.js loader configuration */
-  resource: Resource;
+  /** gtag.js loader configuration */
+  resource?: Resource;
src/utils.ts (1)

58-78: Recursive deep merge implementation

The deepMerge function correctly implements a recursive object merge strategy, properly handling nested objects.

Consider adding protection against circular references and maximum recursion depth to prevent potential stack overflow errors with very deeply nested objects:

+ const MAX_DEPTH = 100;
+ 
+ export function deepMerge<T extends DeepMergeable, U extends DeepMergeable>(
+   target: T,
+   source: U,
+   depth = 0,
+   seen = new WeakMap()
+ ): T & U {
+   if (depth > MAX_DEPTH) {
+     console.warn('Maximum deepMerge depth exceeded');
+     return { ...target, ...source } as T & U;
+   }
+ 
+   // Detect circular references
+   if (isObject(source)) {
+     if (seen.has(source)) {
+       return seen.get(source);
+     }
+     seen.set(source, null);
+   }
  
  const output: DeepMergeable = { ...target };
  
  for (const key in source) {
    if (Object.prototype.hasOwnProperty.call(source, key)) {
      const sourceValue = source[key];
      const targetValue = target[key];
  
      if (isObject(sourceValue) && isObject(targetValue)) {
-       output[key] = deepMerge(targetValue, sourceValue);
+       output[key] = deepMerge(targetValue, sourceValue, depth + 1, seen);
      } else {
        output[key] = sourceValue;
      }
    }
  }
  
+   if (isObject(source)) {
+     seen.set(source, output);
+   }
+   
  return output as T & U;
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1efb5f3 and 49c5acd.

📒 Files selected for processing (5)
  • src/add-gtag.test.ts (1 hunks)
  • src/add-gtag.ts (1 hunks)
  • src/types/settings.ts (1 hunks)
  • src/utils.test.ts (1 hunks)
  • src/utils.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/utils.test.ts
  • src/add-gtag.ts
🔇 Additional comments (23)
src/types/settings.ts (6)

50-75: Review required vs. optional properties in Settings type.

In the Settings type, resource, dataLayerName, and groupName are required properties while tagId is optional. Please verify if this is intentional. Typically, the tag ID would be required for Google Analytics configuration, while other properties might be optional with default values.

Also, consider providing sensible defaults for the required properties to make the library easier to use.


1-4: The imports are well-organized and clearly indicate dependencies.

The file properly imports the necessary types from related modules.


11-14: Good flexibility with PageTrackerTemplate type.

The union type allows for both static and dynamic generation of page tracking parameters, which provides good flexibility to users.


15-32: Well-documented PageTracker type with comprehensive options.

The PageTracker type includes a good set of configuration options with clear JSDoc comments. The callbacks for before and after tracking are particularly useful for advanced use cases.


34-46: Well-structured Resource type with security considerations.

The Resource type includes important properties like nonce for Content Security Policy, which is a good security practice.


5-9:

✅ Verification successful

Check Gtag namespace import.

The Gtag namespace is used in the ConfigParams type but doesn't appear to be imported anywhere in this file. Make sure the namespace is properly defined or imported in your project.


🏁 Script executed:

#!/bin/bash
# Check for Gtag namespace definition
echo "Searching for Gtag namespace definition"
rg "namespace Gtag" --type ts
echo -e "\nSearching for Gtag type imports"
rg "import.+Gtag" --type ts

Length of output: 476


Gtag namespace is properly defined globally

The Gtag namespace is declared in the ambient type file (src/gtag.d.ts), so an explicit import in src/types/settings.ts isn’t necessary. The namespace declaration is being picked up by the TypeScript compiler, which makes the current usage in ConfigParams valid.

src/add-gtag.test.ts (11)

1-16: Nicely structured test file setup

The imports and mocks are properly organized. Using vi.mock with importActual for the utils module ensures you can mock specific functions while keeping the real implementation of others.


17-19: Good test organization with proper test setup

Using beforeEach(resetSettings) ensures each test starts with a clean state, which is essential for test isolation.


20-37: Well-implemented test for basic gtag.js library loading

This test verifies that the library is downloaded with the correct URL parameters when using default configuration.


39-58: Properly tests custom resource configuration

The test correctly verifies that a custom resource URL is respected when specified in settings.


60-71: Callback validation is properly tested

The test verifies that the onReady callback is executed when the plugin is loaded.


73-86: Good error handling test

This test correctly mocks a rejection scenario and verifies that the onError callback is triggered when script loading fails.


88-96: Confirms initial configuration is called

The test verifies that addConfiguration is called during initialization, which is important for establishing the base configuration.


98-115: Router tracking initialization is properly tested

This test ensures that router tracking is initialized when a router instance is provided in the settings.


117-121: Appropriate validation for missing tagId

The test confirms that addConfiguration is not called when tagId is missing, which prevents invalid setup.


123-146: Tests for custom script scenarios are comprehensive

These tests verify both that the script is not injected when useCustomScript is true and that the onReady callback is still triggered in this scenario.


148-165: Security feature tested properly

The test verifies that the nonce attribute is correctly passed to the injectScript function when specified in settings, which is important for Content Security Policy compliance.

src/utils.ts (6)

1-3: Simple and effective server detection

The isServer function provides a straightforward way to check if code is running in a server environment.


5-16: Well-typed script injection function

The function signature with TypeScript typing is clear and provides good documentation of the available options.


18-31: Script element creation is well implemented

The code properly creates and configures a script element with the necessary attributes based on the provided options.


32-42: Resource preconnection is a good performance optimization

Creating a preconnect link for the script's origin can improve loading performance, especially on slower connections.


43-47: Script loading events are properly handled

The code correctly attaches onload and onerror handlers to resolve or reject the promise.


50-56: Good type definition for deepMerge

The DeepMergeable type and isObject helper function are well implemented for the deepMerge functionality.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/add-router-tracking.test.ts (1)

13-92: Consider adding a test for the early return case.

The test suite is comprehensive for the happy path scenarios, but missing a test for when pageTracker.router is undefined, which would trigger the early return in the addRouterTracking function.

it("should do nothing if router is not configured", async () => {
  updateSettings({ pageTracker: { router: undefined } });
  
  await addRouterTracking();
  
  expect(trackRoute).not.toHaveBeenCalled();
});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1748746 and 9fc41cf.

📒 Files selected for processing (3)
  • src/add-router-tracking.test.ts (1 hunks)
  • src/add-router-tracking.ts (1 hunks)
  • src/settings.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/add-router-tracking.ts
🧰 Additional context used
🧬 Code Definitions (2)
src/add-router-tracking.test.ts (2)
src/settings.ts (2)
  • resetSettings (210-212)
  • updateSettings (214-216)
src/add-router-tracking.ts (1)
  • addRouterTracking (4-24)
src/settings.ts (4)
src/api/pageview.ts (1)
  • Pageview (5-5)
src/api/screenview.ts (1)
  • Screenview (4-7)
src/api/linker.ts (1)
  • LinkerParams (3-8)
src/utils.ts (1)
  • deepMerge (70-90)
🔇 Additional comments (12)
src/settings.ts (7)

1-12: Well-structured imports and type aliases

Good job on organizing imports by specific types rather than entire modules, and creating type aliases to centralize Vue Router dependencies. This approach makes the codebase more maintainable if you need to change router implementations in the future.


14-63: Well-documented PageTracker types with flexible configuration options

The PageTracker types are thoroughly documented with JSDoc comments. I particularly like the flexibility provided by allowing both object literals and functions for templates and exclude options, which enhances customization possibilities for users.


65-98: Comprehensive hook system with clear documentation

The hooks implementation provides a robust event system with clear documentation about trigger timing and parameters. This will be valuable for users who need to perform custom actions at specific points in the tracking lifecycle.


100-123: Good resource configuration with sensible defaults

The Resource type provides good flexibility for script loading configuration with clear documentation and sensible defaults. The addition of CSP nonce support is particularly helpful for security-conscious applications.


127-195: Comprehensive Settings type with proper documentation

The Settings type provides a complete configuration API with well-documented options. The distinction between PluginSettings and Settings, making tagId required in the former, is a good design decision to ensure proper initialization.


197-202: Proper use of readonly for default settings

Good job marking defaultSettings as Readonly to prevent accidental modifications. This ensures the defaults remain consistent throughout the application lifecycle.


214-216: Improved settings update with deep merge

Great implementation of updateSettings using deepMerge rather than a shallow merge. This ensures that nested configuration objects are properly merged instead of being overwritten.

src/add-router-tracking.test.ts (5)

1-7: Well-structured test setup with proper mocking.

The imports and mocking setup are clean and appropriate for the tests. Mocking trackRoute is the right approach for unit testing the router tracking functionality in isolation.


8-12: Good minimal route definition for testing.

The routes array provides a simple and adequate setup for testing router functionality without unnecessary complexity.


13-34: First test case correctly verifies initial tracking.

This test effectively verifies that the router tracking initializes correctly by checking that trackRoute is called with the initial route path once the router is ready. Good use of mocking for the isReady method.


35-55: Second test case properly validates tracking after navigation.

The test correctly verifies that route changes trigger tracking by pushing to a new route and checking that trackRoute is called with the new path. The use of toHaveBeenNthCalledWith ensures the proper sequence is verified.


57-91: Third test case correctly verifies duplicate path handling.

This test properly validates that identical consecutive paths don't trigger multiple tracking events, which aligns with the implementation in add-router-tracking.ts that skips tracking when paths are identical.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
src/create-gtag.ts (2)

37-38: Clarify the naming of CreateGtagReturn.

Although CreateGtagReturn is self-descriptive as a function that installs on Vue’s App, consider naming it more explicitly, e.g., InstallFunction or VuePluginInstallFn, so it’s immediately clear to collaborators when reading usage contexts.


39-46: Handle potential issues with updateSettings before script injection.

createGtag updates the settings and immediately calls addGtag() without handling a scenario where tagId might be missing or incorrect. Since PluginSettings requires tagId, this is likely safe. Still, consider adding a runtime check for tagId if you expect dynamic usage or user-provided configuration, to provide earlier error reporting.

src/composable/use-gtag-with-consent.test.ts (2)

29-43: Validate concurrency in asynchronous acceptance.

Observing acceptAll() calls createGtag internally and then reloads the page. Consider whether more granular checks are needed to confirm that the script is fully initialized before reloading, especially in real production usage. The test otherwise is correctly verifying the acceptance flow.


69-79: Edge case: rejecting after consenting.

Currently, the test ensures rejectAll calls consentDeniedAll and doesn’t reload. A nice extension might be verifying the behavior if the user previously provided some form of consent. Although it might be safe, consider adding an extra test to confirm an edge scenario, e.g., partial acceptance before rejection.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9fc41cf and df3705d.

📒 Files selected for processing (4)
  • package.json (4 hunks)
  • src/composable/use-gtag-with-consent.test.ts (1 hunks)
  • src/composable/use-gtag-with-consent.ts (1 hunks)
  • src/create-gtag.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • package.json
  • src/composable/use-gtag-with-consent.ts
🧰 Additional context used
🧬 Code Definitions (2)
src/composable/use-gtag-with-consent.test.ts (2)
src/settings.ts (2)
  • resetSettings (210-212)
  • updateSettings (214-216)
src/composable/use-gtag-with-consent.ts (1)
  • useGtagWithConsent (17-55)
src/create-gtag.ts (1)
src/settings.ts (2)
  • PluginSettings (194-195)
  • updateSettings (214-216)
🔇 Additional comments (8)
src/create-gtag.ts (3)

18-35: Check for possible extension or customization in the api const.

The api object centralizes multiple functionality modules. This design is flexible, but if additional custom methods or relevant hooks need to be exposed later, confirm that updates here maintain backward compatibility. The approach is clean and consistent otherwise.


50-54: Useful module augmentation for Vue.

Explicitly extending ComponentCustomProperties for $gtag is a solid approach, ensuring typed usage in Vue. This is good for plugin-based patterns and keeps code strongly typed across the app.


1-16:

❓ Verification inconclusive

Ensure async call alignment if needed.

Currently, addGtag is imported, but we do not explicitly verify whether addGtag() is synchronous or asynchronous. If addGtag is async, consider using await or explicit error handling here to prevent potential race conditions and to make sure the script is fully loaded before proceeding.

Could you confirm whether addGtag returns a promise or a synchronous operation? If it's async, here’s a sample script to search for its definition and see whether it is declared as async:


🏁 Script executed:

#!/bin/bash
ast-grep --pattern $'function addGtag($_) async {\n  $$$\n}' 

Length of output: 59


Below is a new shell script to further verify whether addGtag is declared as an asynchronous function. Please run these commands and share the output so we can confirm if any async patterns (for a regular function or an arrow function) exist in src/add-gtag.ts:

#!/bin/bash
echo "Checking for async declaration (regular function) in src/add-gtag.ts:"
rg -n "async function addGtag" src/add-gtag.ts

echo ""
echo "Checking for async declaration (arrow function) in src/add-gtag.ts:"
rg -n "const addGtag\s*=\s*async" src/add-gtag.ts

Based on the output:

  • If one of these searches returns a match, it indicates that addGtag is asynchronous and should be invoked with await (along with proper error handling) to prevent race conditions.
  • If no async declaration is found, then the current synchronous usage is likely correct.

Please run these scripts and then manually verify the results before proceeding.

src/composable/use-gtag-with-consent.test.ts (5)

1-12: Mocking strategy is well-defined.

Mocking consent-related APIs, addGtag, and settings ensures isolation of side effects. This is a good practice for test stability and clarity.


17-27: Environment setup is clear.

Resetting settings and stubbing location.reload in beforeEach/afterEach blocks ensures each test starts fresh. This helps avoid cross-test contamination and is a best practice.


45-67: Test coverage for custom consent props is beneficial.

The logic for partial acceptance using distinct property values (ad_storage, ad_user_data) is validated. This ensures the code can set custom consent states. Great job covering this scenario in detail.


81-86: Verifying no call is made when no consent is found.

Good coverage for the case where no _ga cookie is set. The test ensures hasConsent is false and that the analytics script is never added. This aligns with consent prerequisites.


88-99: Assertion clarity for existing consent.

Checking that hasConsent.value is true when _ga exists, along with the subsequent calls, is a thorough approach. It’s well aligned with your default consentMode: "denied" logic and ensures code correctness for returning users.

@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Mar 26, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 11

🧹 Nitpick comments (30)
src/api/time/time.ts (1)

1-6: LGTM! Well-implemented timing event function.

The time function correctly triggers Google Analytics timing events by calling the event function with "timing_complete" as the event name.

-export default function time(params: GtagEventParams) {
+export function time(params: GtagEventParams) {

Consider using a named export instead of a default export to maintain consistency with other modules in the codebase (like customMap in custom-map.ts).

src/core/add-router-tracking.ts (1)

13-15: Consider error handling for router readiness

The function waits for the router to be ready but doesn't include error handling. If router.isReady() rejects, it could cause unhandled promise rejections.

- await router.isReady();
+ try {
+   await router.isReady();
+ } catch (error) {
+   console.error("Failed to initialize router tracking:", error);
+   return;
+ }
src/api/linker/linker.ts (1)

10-12: Consider adding JSDoc comments for better developer experience

The implementation is clean and straightforward, but adding JSDoc comments would improve documentation.

+/**
+ * Configure cross-domain measurement by specifying which domains to link.
+ * @param params - Configuration options for the linker
+ */
 export function linker(params: LinkerParams) {
   query("set", "linker", params);
 }
src/api/time/time.test.ts (1)

1-16: Test effectively validates the time function behavior

The test correctly validates that the time function forwards parameters to the appropriate event type.

However, unlike the custom-map test, this test doesn't call resetSettings in a beforeEach hook. Consider adding it for consistency with other test files if the function uses global settings.

src/api/exception/exception.test.ts (1)

1-18: Test effectively validates the exception function behavior

The test correctly validates that the exception function forwards parameters to the appropriate event type.

As with the time test, consider adding resetSettings in a beforeEach hook for consistency with other test files if the function uses global settings.

src/api/event/event.ts (1)

12-20: Consider creating a copy of the params object to avoid side effects.

The function currently modifies the params object directly when setting the send_to property. This could lead to unexpected behavior if the caller expects the original object to remain unchanged.

export function event(name: GtagEventNames, params: EventParams) {
  const { groupName, additionalAccounts } = getSettings();

-  if (params.send_to === undefined && additionalAccounts?.length) {
-    params.send_to = groupName;
-  }
+  const eventParams = { ...params };
+  
+  if (eventParams.send_to === undefined && additionalAccounts?.length) {
+    eventParams.send_to = groupName;
+  }

-  query("event", name, params);
+  query("event", name, eventParams);
}
src/api/config/config.ts (1)

5-21: Well structured config implementation!

The function effectively configures tracking for both the main tagId and any additional accounts. The early returns are a good practice for avoiding deep nesting.

Consider adding a brief JSDoc comment to document this function's purpose, parameters, and behavior - especially since it's an important part of the tracking architecture.

+/**
+ * Configures Google Analytics tracking with the specified parameters
+ * @param params - Configuration parameters to apply to the tracking ID(s)
+ */
export function config(params: GtagConfig) {
  const { tagId, additionalAccounts } = getSettings();

  if (!tagId) {
    return;
  }

  query("config", tagId, params);

  if (!additionalAccounts) {
    return;
  }

  for (const account of additionalAccounts) {
    query("config", account.tagId, params);
  }
}
src/core/settings.test.ts (1)

3-26: Good test coverage for settings functionality

The tests properly verify both retrieving default settings and updating them with new values. The use of expect.objectContaining is appropriate for partial object matching.

Consider adding a test for the resetSettings function to ensure settings are properly restored to defaults after modification.

import { getSettings, updateSettings, resetSettings } from "./settings";

describe("config", () => {
  it("should return the configuration", () => {
    expect(getSettings()).toEqual({
      resource: { url: "https://www.googletagmanager.com/gtag/js" },
      dataLayerName: "dataLayer",
      gtagName: "gtag",
      groupName: "default",
    });
  });

  it("should update the default configuration", () => {
    updateSettings({
      tagId: "UA-1234567",
      gtagName: "foo",
    });

    expect(getSettings()).toEqual(
      expect.objectContaining({
        tagId: "UA-1234567",
        gtagName: "foo",
      }),
    );
  });
  
+  it("should reset configuration to defaults", () => {
+    // First update settings
+    updateSettings({
+      tagId: "UA-1234567",
+      gtagName: "foo",
+    });
+    
+    // Then reset
+    resetSettings();
+    
+    // Verify defaults are restored
+    expect(getSettings()).toEqual({
+      resource: { url: "https://www.googletagmanager.com/gtag/js" },
+      dataLayerName: "dataLayer",
+      gtagName: "gtag",
+      groupName: "default",
+    });
+  });
});
src/core/add-router-tracking.test.ts (1)

13-92: Comprehensive router tracking tests

The tests cover all key behaviors: initial route tracking, navigation tracking, and skipping duplicate routes. The setup with mock routes and spying on router methods is well implemented.

However, there's repetitive code in the router creation and setup. Consider extracting this into a helper function to improve test maintainability.

describe("add-router-tracking", () => {
  beforeEach(resetSettings);
+  
+  function setupRouter() {
+    const router = createRouter({
+      history: createWebHistory(),
+      routes,
+    });
+    
+    vi.spyOn(router, "isReady").mockResolvedValue();
+    return router;
+  }

  it("should track once the router is ready", async () => {
-    const router = createRouter({
-      history: createWebHistory(),
-      routes,
-    });
-
-    vi.spyOn(router, "isReady").mockResolvedValue();
+    const router = setupRouter();

    updateSettings({ pageTracker: { router } });

    await addRouterTracking();

    expect(trackRoute).toHaveBeenCalledWith(
      expect.objectContaining({
        path: "/",
      }),
    );
  });

  it("should track after route change", async () => {
-    const router = createRouter({
-      history: createWebHistory(),
-      routes,
-    });
-
-    vi.spyOn(router, "isReady").mockResolvedValue();
+    const router = setupRouter();

    updateSettings({ pageTracker: { router } });

    await addRouterTracking();

    await router.push("/about");

    expect(trackRoute).toHaveBeenNthCalledWith(
      2,
      expect.objectContaining({
        path: "/about",
      }),
    );
  });

  it("should skip tracking identical paths", async () => {
-    const router = createRouter({
-      history: createWebHistory(),
-      routes,
-    });
-
-    vi.spyOn(router, "isReady").mockResolvedValue();
+    const router = setupRouter();

    updateSettings({
      pageTracker: {
        router,
      },
    });

    await addRouterTracking();

    await router.push("/about");
    await router.push("/about");

    expect(trackRoute).toHaveBeenNthCalledWith(
      1,
      expect.objectContaining({
        path: "/",
      }),
    );

    expect(trackRoute).toHaveBeenNthCalledWith(
      2,
      expect.objectContaining({
        path: "/about",
      }),
    );

    expect(trackRoute).toHaveBeenCalledTimes(2);
  });
});
src/api/event/event.test.ts (1)

7-57: Good test coverage for event functionality

The tests effectively verify the core behaviors of the event function, including proper handling of the send_to property across different scenarios.

To improve test maintainability, consider using a beforeEach hook to reset settings before each test, as you've done in other test files. This would prevent potential test interdependencies.

import { updateSettings, resetSettings } from "../../core/settings";
import { query } from "../query";
import { event } from "./event";

vi.mock("../query");

describe("event", () => {
+  beforeEach(resetSettings);
+
  it("should query the event command", () => {
    event("screen_view", { screen_name: "about" });

    expect(query).toHaveBeenCalledWith("event", "screen_view", {
      screen_name: "about",
    });
  });

  it("should add a group name", () => {
    updateSettings({
      additionalAccounts: [{ tagId: "UA-1" }, { tagId: "UA-2" }],
    });

    event("screen_view", { screen_name: "about" });

    expect(query).toHaveBeenCalledWith("event", "screen_view", {
      screen_name: "about",
      send_to: "default",
    });
  });

  it("should use a custom group name", () => {
    updateSettings({
      additionalAccounts: [{ tagId: "UA-1" }, { tagId: "UA-2" }],
      groupName: "custom_group_name",
    });

    event("screen_view", { screen_name: "about" });

    expect(query).toHaveBeenCalledWith("event", "screen_view", {
      screen_name: "about",
      send_to: "custom_group_name",
    });
  });

  it("should use the send_to property if already set", () => {
    updateSettings({
      additionalAccounts: [{ tagId: "UA-1" }, { tagId: "UA-2" }],
    });

    event("screen_view", {
      screen_name: "about",
      send_to: "my_custom_send_to",
    });

    expect(query).toHaveBeenCalledWith("event", "screen_view", {
      screen_name: "about",
      send_to: "my_custom_send_to",
    });
  });
});
src/core/track-route.test.ts (1)

201-221: Consider adding a test for combined custom template and screenview.

The test for custom template with screenview is good, but consider adding another test that uses a function template with screenview to fully cover all permutations.

 it("should track screen views using a custom template", async () => {
   updateSettings({
     pageTracker: {
       router,
       useScreenview: true,
       template: {
         screen_name: "about",
       },
     },
   });

   await router.isReady();
   await router.push("/");

   trackRoute(router.currentRoute.value);

   expect(query).toHaveBeenCalledWith("event", "screen_view", {
     screen_name: "about",
   });
 });
+
+it("should track screen views using a custom template function", async () => {
+  updateSettings({
+    appName: "MyApp",
+    pageTracker: {
+      router,
+      useScreenview: true,
+      template: (to) => ({
+        screen_name: `${to.name}_custom`,
+      }),
+    },
+  });
+
+  await router.isReady();
+  await router.push("/about");
+
+  trackRoute(router.currentRoute.value);
+
+  expect(query).toHaveBeenCalledWith("event", "screen_view", {
+    screen_name: "about_custom",
+    app_name: "MyApp",
+  });
+});
src/api/opt/opt.test.ts (1)

4-9: Consider adding error case tests.

The tests cover the main functionality well, but consider adding tests for error cases, such as invalid tag IDs or edge cases.

 beforeEach(() => {
   resetSettings();
   window["ga-disable-UA-1"] = undefined;
   window["ga-disable-UA-2"] = undefined;
 });
+
+it("should handle invalid tag IDs gracefully", () => {
+  // Test with empty string
+  optOut("");
+  expect(window["ga-disable-"]).toBeUndefined();
+  
+  // Test with null value (assuming TypeScript allows this test)
+  optOut(null as any);
+  expect(window["ga-disable-null"]).toBeUndefined();
+});
src/api/query/query.ts (3)

5-12: Consider more specific type for Window interface extension.

While the dynamic indexing with [key: string] works, it might be more type-safe to limit this to known property patterns for tracking (like 'ga-disable-*' properties).

 declare global {
   interface Window {
     // biome-ignore lint/suspicious/noExplicitAny:
-    [key: string]: any | any[];
+    [key: `${string}`]: any | any[];
+    // More specific for tracking properties
+    [key: `ga-disable-${string}`]?: boolean;
+    [key: `${string}DataLayer`]?: any[];
   }
 }

14-30: Avoid redefining window[gtagName] function on every query call.

The current implementation redefines the global gtag function on every call to query, which is inefficient for repeated calls. Consider defining this function once during initialization.

 export function query(...args: QueryParams) {
   const { dataLayerName, gtagName } = getSettings();
 
   if (isServer()) {
     return;
   }
 
   window[dataLayerName] = window[dataLayerName] || [];
 
-  // biome-ignore lint/complexity/useArrowFunction:
-  window[gtagName] = function () {
-    // biome-ignore lint/style/noArguments:
-    window[dataLayerName].push(arguments);
-  };
+  // Only define the function if it doesn't exist yet
+  if (typeof window[gtagName] !== 'function') {
+    // biome-ignore lint/complexity/useArrowFunction:
+    window[gtagName] = function () {
+      // biome-ignore lint/style/noArguments:
+      window[dataLayerName].push(arguments);
+    };
+  }
 
   window[gtagName](...args);
 }

17-19: Consider providing more context for isServer early returns.

The early return when running on the server is good, but consider adding a debug log or comment to explain why tracking is skipped on the server.

 if (isServer()) {
+  // Skip tracking on server-side as window object is not available
   return;
 }
src/api/screenview/screenview.ts (2)

11-29: Improve type safety in screenview function.

The function handles different parameter types, but the type checking could be improved to avoid potential runtime errors.

 export function screenview(params: ScreenviewParams) {
   const { appName } = getSettings();
 
   let template: Screenview = {};
 
   if (typeof params === "string") {
     template.screen_name = params;
-  } else if ("path" in params) {
-    template.screen_name = (params.name ?? params.path) as string;
+  } else if ("path" in params && params.path !== undefined) {
+    // More explicit check for Route type
+    const routeName = params.name !== undefined ? String(params.name) : undefined;
+    template.screen_name = routeName ?? params.path;
   } else {
+    // Must be Screenview type
     template = params;
   }
 
-  if (appName && template?.app_name === undefined) {
+  // template is never undefined here, so no need for optional chaining
+  if (appName && template.app_name === undefined) {
     template.app_name = appName;
   }
 
   query("event", "screen_view", template);
 }

11-29: Consider adding validation for screen_name.

The function doesn't validate that a screen_name exists before sending the event, which could lead to tracking events with missing data.

 export function screenview(params: ScreenviewParams) {
   const { appName } = getSettings();
 
   let template: Screenview = {};
 
   if (typeof params === "string") {
     template.screen_name = params;
   } else if ("path" in params) {
     template.screen_name = (params.name ?? params.path) as string;
   } else {
     template = params;
   }
 
   if (appName && template?.app_name === undefined) {
     template.app_name = appName;
   }
 
+  // Validate that we have a screen name before sending
+  if (!template.screen_name) {
+    console.warn('screenview called without a screen_name');
+    return;
+  }
+
   query("event", "screen_view", template);
 }
src/core/track-route.ts (1)

16-18: Consider simplifying the route exclusion check

The array check could be more concise and avoid potential issues with destructuring undefined objects.

-  return pageTracker.exclude?.some(({ name, path } = {}) => {
-    return (name && name === route.name) || (path && path === route.path);
-  });
+  return pageTracker.exclude?.some((exclusion) => {
+    return (exclusion.name && exclusion.name === route.name) || 
+           (exclusion.path && exclusion.path === route.path);
+  });
src/api/consent/consent.ts (1)

1-24: Consider adding individual consent type management

The current implementation only provides all-or-nothing consent management. Consider adding functions to manage individual consent types for more granular control.

 import type { GtagConsentArg, GtagConsentParams } from "../../types/gtag";
 import { query } from "../query";
 
 export function consent(consentArg: GtagConsentArg, params: GtagConsentParams) {
   query("consent", consentArg, params);
 }
 
 export function consentGrantedAll(mode: GtagConsentArg = "default") {
   consent(mode, {
     ad_user_data: "granted",
     ad_personalization: "granted",
     ad_storage: "granted",
     analytics_storage: "granted",
   });
 }
 
 export function consentDeniedAll(mode: GtagConsentArg = "default") {
   consent(mode, {
     ad_user_data: "denied",
     ad_personalization: "denied",
     ad_storage: "denied",
     analytics_storage: "denied",
   });
 }
+
+// Individual consent type management functions
+export function setAdUserDataConsent(
+  status: "granted" | "denied", 
+  mode: GtagConsentArg = "default"
+) {
+  consent(mode, { ad_user_data: status });
+}
+
+export function setAdPersonalizationConsent(
+  status: "granted" | "denied", 
+  mode: GtagConsentArg = "default"
+) {
+  consent(mode, { ad_personalization: status });
+}
+
+export function setAdStorageConsent(
+  status: "granted" | "denied", 
+  mode: GtagConsentArg = "default"
+) {
+  consent(mode, { ad_storage: status });
+}
+
+export function setAnalyticsStorageConsent(
+  status: "granted" | "denied", 
+  mode: GtagConsentArg = "default"
+) {
+  consent(mode, { analytics_storage: status });
+}
src/core/add-configuration.test.ts (1)

13-25: Consider parameterizing similar test cases

The test structure is duplicated across several test cases. Consider using a parameterized test approach to reduce duplication.

- it("should query with initial configuration", () => {
-   updateSettings({
-     tagId: "UA-12345678",
-   });
-
-   addConfiguration();
-
-   expect(query).toHaveBeenNthCalledWith(1, "js", new Date());
-   expect(query).toHaveBeenNthCalledWith(2, "config", "UA-12345678", {
-     send_page_view: false,
-     anonymize_ip: true,
-   });
- });
+ const testCases = [
+   {
+     name: "initial configuration",
+     settings: { tagId: "UA-12345678" },
+     expectedCalls: [
+       { nth: 1, args: ["js", new Date()] },
+       { 
+         nth: 2, 
+         args: ["config", "UA-12345678", {
+           send_page_view: false,
+           anonymize_ip: true,
+         }]
+       }
+     ]
+   },
+   // Add more test cases here
+ ];
+
+ testCases.forEach(({ name, settings, expectedCalls }) => {
+   it(`should query with ${name}`, () => {
+     updateSettings(settings);
+     addConfiguration();
+     expectedCalls.forEach(({ nth, args }) => {
+       expect(query).toHaveBeenNthCalledWith(nth, ...args);
+     });
+   });
+ });
src/core/add-gtag.ts (2)

13-17: Consider changing the order of operations

The configuration and router tracking are set up before checking if Gtag already exists. Consider checking for Gtag existence first to avoid unnecessary configuration if the script is already loaded.

 export default async function addGtag(): Promise<void> {
   const { resource, dataLayerName, tagId, pageTracker, hooks } = getSettings();

   if (!tagId) {
     return;
   }

+  if (hasGtag()) {
+    hooks?.["script:loaded"]?.();
+    addConfiguration();
+    
+    if (pageTracker?.router) {
+      addRouterTracking();
+    }
+    return;
+  }
+
   addConfiguration();

   if (pageTracker?.router) {
     addRouterTracking();
   }

-  if (hasGtag()) {
-    hooks?.["script:loaded"]?.();
-    return;
-  }
-
   try {
     await injectScript(`${resource.url}?id=${tagId}&l=${dataLayerName}`, {
       preconnect: resource.preconnect,
       defer: resource.defer,
       nonce: resource.nonce,
     });

     hooks?.["script:loaded"]?.();
   } catch (error) {
     hooks?.["script:error"]?.(error);
   }
 }

32-34: Consider adding more specific error handling

The current error handling doesn't distinguish between different types of errors that could occur during script injection. Consider adding more specific error handling or logging.

   try {
     await injectScript(`${resource.url}?id=${tagId}&l=${dataLayerName}`, {
       preconnect: resource.preconnect,
       defer: resource.defer,
       nonce: resource.nonce,
     });

     hooks?.["script:loaded"]?.();
   } catch (error) {
+    console.error("Failed to load Google Analytics script:", error);
     hooks?.["script:error"]?.(error);
   }
src/api/consent/consent.test.ts (1)

1-62: Test coverage looks good but could be enhanced with edge cases.

The test suite effectively verifies the core functionality of consent management functions, confirming that each function properly calls the query method with the expected parameters. The mocking approach is appropriate.

I would suggest adding a few more test cases:

  1. Test error handling scenarios
  2. Edge cases with partial consent parameters
  3. Verify behavior with invalid consent argument values

Add tests for edge cases:

it("should handle partial consent parameters", () => {
  consent("update", {
    analytics_storage: "granted",
    // Only specifying one parameter
  });

  expect(query).toHaveBeenCalledWith("consent", "update", {
    analytics_storage: "granted",
  });
});

it("should handle invalid consent arguments gracefully", () => {
  // @ts-expect-error: Testing with invalid argument
  consent("invalid_argument", {
    ad_storage: "granted",
  });

  expect(query).toHaveBeenCalledWith("consent", "invalid_argument", {
    ad_storage: "granted",
  });
});
src/api/screenview/screenview.test.ts (3)

1-26: Good test setup but consider mock restoration.

The test setup is comprehensive, but you should reset your mocks between tests to ensure test isolation.

Add cleanup after each test:

beforeEach(async () => {
  resetSettings();
  
  router = createRouter({
    history: createWebHistory("/base-path"),
    routes: [
      { path: "/no-name", component: { template: "<div />" } },
      { path: "/about", name: "about", component: { template: "<div />" } },
    ],
  });
  
  vi.spyOn(router, "isReady").mockResolvedValue();
  
  await router.isReady();
  await router.push({ name: "about", query: { id: 1 }, hash: "#title" });
});

+ afterEach(() => {
+   vi.resetAllMocks();
+ });

28-46: Add negative test case for invalid screen name parameter.

The tests cover the happy path scenarios but lack tests for invalid parameter handling.

Add a test for invalid screen name parameter:

it("should handle undefined screen name gracefully", () => {
  // @ts-expect-error Testing invalid parameter
  screenview(undefined);
  
  // Expect query not to be called or called with default values
  expect(query).toHaveBeenCalledWith("event", "screen_view", expect.any(Object));
});

48-84: Consider testing error scenarios in route tracking.

The pageTracker tests are good but should also test error scenarios to ensure robustness.

Add a test for route tracking failures:

it("should handle route tracking errors gracefully", async () => {
  // Mock router to throw an error
  const originalRoute = router.currentRoute;
  Object.defineProperty(router, 'currentRoute', {
    get: () => {
      throw new Error('Route error');
    }
  });
  
  try {
    // This should not throw
    screenview(router);
    
    // Expect query not to be called because of the error
    expect(query).not.toHaveBeenCalled();
  } finally {
    // Restore router
    Object.defineProperty(router, 'currentRoute', {
      get: () => originalRoute.value
    });
  }
});
src/composables/use-consent/use-consent.ts (1)

19-20: Make cookie name a configurable constant.

The GA cookie name is hardcoded, which could become problematic if the cookie name changes in the future or if a different implementation uses a different name.

Move the cookie name to a configurable constant:

-const GA_COOKIE_VALUE = "_ga";
+/**
+ * Default cookie name for Google Analytics
+ * @type {string}
+ */
+const DEFAULT_GA_COOKIE_VALUE = "_ga";

Then update the usage to allow for configuration through settings:

const hasConsent = ref<boolean>(
- isServer() ? false : document.cookie.includes(GA_COOKIE_VALUE),
+ isServer() ? false : document.cookie.includes(settings.gaCookieName || DEFAULT_GA_COOKIE_VALUE),
);
src/api/pageview/pageview.ts (2)

9-14: Consider edge case in path normalization.

The getPathWithBase function handles most path combinations well, but has an edge case when both base and path are "/", resulting in "//".

Consider this improved implementation:

function getPathWithBase(path: string, base: string): string {
  const normalizedBase = base.endsWith("/") ? base : `${base}/`;
  const normalizedPath = path.startsWith("/") ? path.substring(1) : path;

- return `${normalizedBase}${normalizedPath}`;
+ // Avoid double slash when normalized path is empty
+ return normalizedPath ? `${normalizedBase}${normalizedPath}` : normalizedBase;
}

21-37: Type casting concern in route name handling.

The code assumes params.name is a string when present, but there's no type check before casting.

Consider adding a more explicit type check:

template = {
-  ...(params.name ? { page_title: params.name as string } : {}),
+  ...(params.name && typeof params.name === 'string' ? { page_title: params.name } : {}),
  page_path: pageTracker?.useRouterBasePath
    ? getPathWithBase(path, base)
    : path,
};
src/types/gtag.ts (1)

46-46: Refine the callback parameter type.
Using any in callback?: (field?: string | GtagCustomParams) => any might reduce type safety. Consider narrowing the return type or using unknown for improved clarity.

-    callback?: (field?: string | GtagCustomParams) => any,
+    callback?: (field?: string | GtagCustomParams) => unknown,
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6809274 and 012973a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (59)
  • biome.json (1 hunks)
  • package.json (3 hunks)
  • src/api/config/config.test.ts (1 hunks)
  • src/api/config/config.ts (1 hunks)
  • src/api/config/index.ts (1 hunks)
  • src/api/consent/consent.test.ts (1 hunks)
  • src/api/consent/consent.ts (1 hunks)
  • src/api/consent/index.ts (1 hunks)
  • src/api/custom-map/custom-map.test.ts (1 hunks)
  • src/api/custom-map/custom-map.ts (1 hunks)
  • src/api/custom-map/index.ts (1 hunks)
  • src/api/ecommerce/ecommerce.test.ts (1 hunks)
  • src/api/ecommerce/ecommerce.ts (1 hunks)
  • src/api/ecommerce/index.ts (1 hunks)
  • src/api/event/event.test.ts (1 hunks)
  • src/api/event/event.ts (1 hunks)
  • src/api/event/index.ts (1 hunks)
  • src/api/exception/exception.test.ts (1 hunks)
  • src/api/exception/exception.ts (1 hunks)
  • src/api/exception/index.ts (1 hunks)
  • src/api/index.ts (1 hunks)
  • src/api/linker/index.ts (1 hunks)
  • src/api/linker/linker.test.ts (1 hunks)
  • src/api/linker/linker.ts (1 hunks)
  • src/api/opt/index.ts (1 hunks)
  • src/api/opt/opt.test.ts (1 hunks)
  • src/api/opt/opt.ts (1 hunks)
  • src/api/pageview/index.ts (1 hunks)
  • src/api/pageview/pageview.test.ts (1 hunks)
  • src/api/pageview/pageview.ts (1 hunks)
  • src/api/query/index.ts (1 hunks)
  • src/api/query/query.test.ts (1 hunks)
  • src/api/query/query.ts (1 hunks)
  • src/api/screenview/index.ts (1 hunks)
  • src/api/screenview/screenview.test.ts (1 hunks)
  • src/api/screenview/screenview.ts (1 hunks)
  • src/api/set/index.ts (1 hunks)
  • src/api/set/set.test.ts (1 hunks)
  • src/api/set/set.ts (1 hunks)
  • src/api/time/index.ts (1 hunks)
  • src/api/time/time.test.ts (1 hunks)
  • src/api/time/time.ts (1 hunks)
  • src/composables/index.ts (1 hunks)
  • src/composables/use-consent/index.ts (1 hunks)
  • src/composables/use-consent/use-consent.test.ts (1 hunks)
  • src/composables/use-consent/use-consent.ts (1 hunks)
  • src/core/add-configuration.test.ts (1 hunks)
  • src/core/add-configuration.ts (1 hunks)
  • src/core/add-gtag.test.ts (1 hunks)
  • src/core/add-gtag.ts (1 hunks)
  • src/core/add-router-tracking.test.ts (1 hunks)
  • src/core/add-router-tracking.ts (1 hunks)
  • src/core/settings.test.ts (1 hunks)
  • src/core/settings.ts (1 hunks)
  • src/core/track-route.test.ts (1 hunks)
  • src/core/track-route.ts (1 hunks)
  • src/index.test.ts (1 hunks)
  • src/index.ts (1 hunks)
  • src/types/gtag.ts (1 hunks)
✅ Files skipped from review due to trivial changes (16)
  • src/api/ecommerce/index.ts
  • src/api/event/index.ts
  • src/api/set/index.ts
  • src/api/linker/index.ts
  • src/api/pageview/index.ts
  • src/api/time/index.ts
  • src/api/query/index.ts
  • src/api/config/index.ts
  • src/composables/index.ts
  • src/api/consent/index.ts
  • src/composables/use-consent/index.ts
  • src/api/custom-map/index.ts
  • src/api/opt/index.ts
  • src/api/screenview/index.ts
  • src/api/exception/index.ts
  • src/api/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • biome.json
  • src/index.test.ts
🧰 Additional context used
🧬 Code Definitions (35)
src/api/custom-map/custom-map.ts (1)
src/api/config/config.ts (1)
  • config (5-21)
src/api/config/config.test.ts (2)
src/api/config/config.ts (1)
  • config (5-21)
src/api/query/query.ts (1)
  • query (14-30)
src/api/time/time.ts (1)
src/api/event/event.ts (1)
  • event (12-20)
src/api/ecommerce/ecommerce.test.ts (2)
src/api/ecommerce/ecommerce.ts (1)
  • ecommerce (7-12)
src/api/query/query.ts (1)
  • query (14-30)
src/api/set/set.test.ts (2)
src/api/set/set.ts (1)
  • set (6-8)
src/api/query/query.ts (1)
  • query (14-30)
src/api/custom-map/custom-map.test.ts (2)
src/api/custom-map/custom-map.ts (1)
  • customMap (4-8)
src/api/query/query.ts (1)
  • query (14-30)
src/api/exception/exception.ts (1)
src/api/event/event.ts (1)
  • event (12-20)
src/api/event/event.test.ts (2)
src/api/event/event.ts (1)
  • event (12-20)
src/api/query/query.ts (1)
  • query (14-30)
src/api/config/config.ts (1)
src/api/query/query.ts (1)
  • query (14-30)
src/api/set/set.ts (1)
src/api/query/query.ts (1)
  • query (14-30)
src/core/add-router-tracking.test.ts (3)
src/core/settings.ts (2)
  • resetSettings (210-212)
  • updateSettings (214-216)
src/core/add-router-tracking.ts (1)
  • addRouterTracking (4-24)
src/core/track-route.ts (1)
  • trackRoute (21-52)
src/api/event/event.ts (1)
src/api/query/query.ts (1)
  • query (14-30)
src/api/linker/linker.ts (1)
src/api/query/query.ts (1)
  • query (14-30)
src/core/settings.test.ts (1)
src/core/settings.ts (2)
  • getSettings (206-208)
  • updateSettings (214-216)
src/api/screenview/screenview.test.ts (2)
src/api/screenview/screenview.ts (1)
  • screenview (11-29)
src/api/query/query.ts (1)
  • query (14-30)
src/api/time/time.test.ts (2)
src/api/time/time.ts (1)
  • time (4-6)
src/api/query/query.ts (1)
  • query (14-30)
src/api/ecommerce/ecommerce.ts (1)
src/api/event/event.ts (1)
  • event (12-20)
src/core/track-route.ts (3)
src/core/settings.ts (3)
  • Route (12-12)
  • getSettings (206-208)
  • PageTrackerParams (14-14)
src/api/screenview/screenview.ts (1)
  • screenview (11-29)
src/api/pageview/pageview.ts (1)
  • pageview (16-48)
src/index.ts (2)
src/core/settings.ts (2)
  • PluginSettings (194-195)
  • updateSettings (214-216)
src/core/add-gtag.ts (1)
  • addGtag (6-35)
src/api/linker/linker.test.ts (2)
src/api/linker/linker.ts (1)
  • linker (10-12)
src/api/query/query.ts (1)
  • query (14-30)
src/api/screenview/screenview.ts (1)
src/api/query/query.ts (1)
  • query (14-30)
src/api/query/query.test.ts (1)
src/api/query/query.ts (1)
  • query (14-30)
src/composables/use-consent/use-consent.ts (1)
src/index.ts (1)
  • createGtag (16-23)
src/composables/use-consent/use-consent.test.ts (1)
src/composables/use-consent/use-consent.ts (1)
  • useConsent (21-57)
src/core/add-router-tracking.ts (2)
src/core/settings.ts (1)
  • getSettings (206-208)
src/core/track-route.ts (1)
  • trackRoute (21-52)
src/api/pageview/pageview.test.ts (2)
src/api/pageview/pageview.ts (1)
  • pageview (16-48)
src/api/query/query.ts (1)
  • query (14-30)
src/core/add-configuration.test.ts (3)
src/core/settings.ts (2)
  • resetSettings (210-212)
  • updateSettings (214-216)
src/core/add-configuration.ts (1)
  • addConfiguration (15-59)
src/api/query/query.ts (1)
  • query (14-30)
src/api/exception/exception.test.ts (2)
src/api/exception/exception.ts (1)
  • exception (11-13)
src/api/query/query.ts (1)
  • query (14-30)
src/api/opt/opt.test.ts (1)
src/api/opt/opt.ts (2)
  • optOut (23-25)
  • optIn (33-35)
src/core/add-configuration.ts (4)
src/core/settings.ts (1)
  • getSettings (206-208)
src/api/consent/consent.ts (2)
  • consentGrantedAll (8-15)
  • consentDeniedAll (17-24)
src/api/linker/linker.ts (1)
  • linker (10-12)
src/api/query/query.ts (1)
  • query (14-30)
src/core/add-gtag.ts (3)
src/core/settings.ts (1)
  • getSettings (206-208)
src/core/add-configuration.ts (1)
  • addConfiguration (15-59)
src/core/add-router-tracking.ts (1)
  • addRouterTracking (4-24)
src/core/add-gtag.test.ts (4)
src/core/settings.ts (2)
  • resetSettings (210-212)
  • updateSettings (214-216)
src/core/add-gtag.ts (1)
  • addGtag (6-35)
src/core/add-configuration.ts (1)
  • addConfiguration (15-59)
src/core/add-router-tracking.ts (1)
  • addRouterTracking (4-24)
src/api/consent/consent.test.ts (2)
src/api/consent/consent.ts (3)
  • consent (4-6)
  • consentGrantedAll (8-15)
  • consentDeniedAll (17-24)
src/api/query/query.ts (1)
  • query (14-30)
src/api/consent/consent.ts (1)
src/api/query/query.ts (1)
  • query (14-30)
src/api/pageview/pageview.ts (1)
src/api/query/query.ts (1)
  • query (14-30)
🔇 Additional comments (89)
src/api/custom-map/custom-map.ts (1)

1-8: LGTM! Well-implemented Google Analytics custom dimensions mapping.

The customMap function is a clean wrapper around the config function that properly sets up custom dimension mapping in Google Analytics. The implementation correctly passes the custom parameters to the underlying configuration system.

src/api/set/set.test.ts (1)

1-14: LGTM! Well-structured and focused test.

The test correctly verifies that the set function passes its parameters to the query function with "set" as the command name, which is the expected behavior.

src/api/exception/exception.ts (1)

1-13: LGTM! Well-implemented exception tracking function with good type definitions.

The exception function correctly triggers Google Analytics exception events with appropriate parameters. The type definition for ExceptionParams provides flexibility by accepting either specific exception properties or custom parameters.

src/api/set/set.ts (1)

1-8: Well-structured type-safe implementation of the set command

The set function provides a clean wrapper around the query mechanism, with proper TypeScript typing using the GtagCommands["set"] type. This maintains type safety while allowing for variable arguments.

src/api/ecommerce/ecommerce.ts (1)

1-12: Clean implementation with proper type safety

The ecommerce function provides a well-typed wrapper around the event function, specifically for ecommerce events. The type definitions ensure type safety while maintaining a clean API.

src/core/add-router-tracking.ts (1)

4-24: Well-implemented router tracking setup

The addRouterTracking function correctly sets up tracking for both the initial route and subsequent navigation. The early return when no router is available is good defensive programming, and the check to avoid tracking when paths haven't changed is an excellent optimization.

src/api/custom-map/custom-map.test.ts (1)

1-23: Well-structured test with good isolation practices

The test correctly validates that the customMap function passes the appropriate parameters to the query function. Using beforeEach(resetSettings) ensures proper test isolation.

src/api/linker/linker.ts (1)

3-8: Well-defined type with appropriate constraints

The LinkerParams type is well-structured with clear optional properties and a precise union type for url_position.

src/api/query/query.test.ts (1)

4-15: Well-structured test for the query function.

This test appropriately verifies that the query function correctly passes events to the gtag library by checking the arguments pushed to the data layer.

src/api/config/config.test.ts (2)

7-18: Comprehensive test for single account configuration.

The test correctly verifies that the config function passes the appropriate parameters to the query function when a single tag ID is configured.


20-39: Good test coverage for multiple account configurations.

This test properly validates that the config function handles multiple Google Analytics accounts by calling query for each configured account with the correct parameters.

src/api/linker/linker.test.ts (1)

6-15: Test adequately verifies linker functionality.

The test ensures that the linker function correctly passes the domains to the Google Analytics linker feature via the query function.

src/core/track-route.test.ts (6)

1-33: Well-structured test setup and initialization.

The test setup is comprehensive, with proper router initialization, route definitions, and mock configurations. The beforeEach hook ensures a clean testing environment for each test case.


35-53: Good implementation of screenview tracking test.

This test effectively verifies that screenviews are tracked correctly with the appropriate parameters, including screen_name and app_name.


55-67: Well-implemented pageview tracking test.

The test appropriately verifies that pageviews include all required parameters: page_path, page_location, page_title, and send_page_view flag.


69-115: Comprehensive route exclusion testing.

These tests thoroughly cover route exclusions using different approaches (path, name, and custom function), ensuring flexibility in configuration.


117-155: Thorough testing of tracking hooks.

The hook tests properly verify that callbacks are fired at the right moments in the tracking flow, enabling custom integration logic.


157-199: Well-implemented custom template testing.

These tests thoroughly validate both static and dynamic template configuration options, ensuring flexibility in customizing tracking data.

src/api/opt/opt.test.ts (4)

1-10: Well-structured test setup and initialization.

The test setup properly resets settings and clears window properties before each test, ensuring a clean testing environment.


11-30: Good tests for basic opt-out and opt-in functionality.

These tests effectively verify the core functionality of opting out and opting back in for analytics tracking.


32-45: Good tests for custom tagId handling.

These tests properly verify that custom tag IDs work correctly with the opt-out and opt-in functions.


47-73: Thorough testing of additional account handling.

The tests for additional accounts verify that both the primary account and additional accounts are correctly opted out and in.

src/api/screenview/screenview.ts (1)

4-9: Well-defined and clear type definitions.

The type definitions are clear and concise, providing a good structure for screen view data.

src/index.ts (3)

1-5: Imports look well-structured and consistent.
Using import type for App aligns with modern TypeScript best practices, minimizing runtime overhead to type-only imports.


8-12: Vue module augmentation is correctly scoped.
Defining $gtag in ComponentCustomProperties ensures that the Gtag API is readily available in templates and components without additional typings.


25-26: Re-exports enhance discoverability.
Re-exporting from ./api/index and ./composables/index makes your library’s interface more intuitive for consumers.

src/core/add-configuration.ts (2)

7-13: mergeDefaults function is concise and clear.
Merging default settings with user overrides in a single expression enhances readability and maintainability.


15-59: Ensure robust handling for unrecognized consentMode values and missing tagId in additionalAccounts.

  1. consentMode only checks "granted" or "denied", ignoring other potential values.
  2. Loop through additionalAccounts assumes each has a tagId. Consider graceful handling or validation for missing IDs.

Would you like a script to search for usage of consentMode or check that all additional accounts supply a valid tagId?

src/api/pageview/pageview.test.ts (8)

1-7: Mocking strategy is straightforward.
Mocking query ensures the test can verify calls to the analytics layer.


8-37: Basic page path tracking tests are clear and well-implemented.
These tests help confirm that passing a string path triggers the correct page_path property in the event.


39-49: Route-based page path handling is validated.
This test confirms that extracting the path from router.currentRoute.value is handled as expected.


51-61: Ensures default page_location.
Verifying that the test environment’s URL is included is a thorough approach to confirm defaults are appended.


63-73: Verifies default send_page_view setting.
Ensuring send_page_view = true by default covers a common usage scenario without further configuration.


75-85: Validates page_title from route naming.
Confirming route name mapping to page_title ensures consistent analytics naming.


87-99: Confirming custom parameters.
This test ensures advanced usage scenarios (e.g., custom location or disabling send_page_view) are correctly handled.


101-176: pageTracker tests thoroughly validate advanced routing scenarios.
The suite covers base paths, full paths, and toggling sendPageView, demonstrating robust coverage for route-based analytics.

src/core/track-route.ts (3)

5-19: Well-structured route exclusion logic

The isRouteExcluded function appropriately handles multiple exclusion scenarios:

  1. No exclusions defined
  2. Function-based exclusion
  3. Array-based exclusions by name or path

21-52: Well-implemented route tracking with flexible template handling

The trackRoute function effectively:

  1. Respects route exclusions
  2. Handles hook callbacks before and after tracking
  3. Supports both static and function-based templates
  4. Conditionally uses either screenview or pageview tracking

40-49: Robust parameter selection for analytics calls

The code correctly handles different parameter sources for both screenview and pageview calls, checking if the template has the expected properties before falling back to using the route directly.

src/api/consent/consent.ts (3)

4-6: Clear wrapper function for consent management

The consent function provides a clean interface to the underlying query mechanism, making the consent API more intuitive to use.


8-15: Convenient helper for granting all consent types

The consentGrantedAll function provides a useful shorthand for the common operation of granting all consent types, with flexibility to override the default mode.


17-24: Convenient helper for denying all consent types

The consentDeniedAll function provides a useful shorthand for the common operation of denying all consent types, with flexibility to override the default mode.

src/core/add-configuration.test.ts (5)

1-26: Well-structured initial configuration test

The test properly mocks dependencies, sets up a fixed date for deterministic testing, and verifies that the query function is called with the expected parameters.


27-63: Comprehensive test for multiple domain configuration

The test thoroughly verifies that multiple domains are configured correctly with their respective settings, ensuring that:

  1. The base configuration is applied
  2. Group names are correctly added
  3. Custom configurations override defaults

65-79: Well-designed linker test

The test verifies that the linker is correctly set up when specified in the configuration.


81-113: Good test coverage for hooks

The tests properly verify that both pre and post hooks are called at the appropriate times relative to the query operation.


115-151: Thorough consent mode testing

The tests verify that the consent settings are correctly applied for both granted and denied modes, including checking the order of operations.

src/core/add-gtag.ts (2)

6-11: Good early return pattern with tag ID check

The function properly handles the case where no tag ID is provided by returning early, preventing unnecessary operations.


24-34: Well-implemented script injection with error handling

The code effectively:

  1. Uses async/await for script injection
  2. Provides necessary parameters from settings
  3. Handles success with appropriate hook
  4. Catches and reports errors through hooks
src/composables/use-consent/use-consent.test.ts (7)

1-17: Proper test setup with mocks and imports.

The test file correctly imports and mocks all necessary dependencies. The mocking strategy is appropriate, especially using vi.importActual to retain the actual implementation while overriding specific functions.


18-32: Well-structured test setup and teardown hooks.

The test suite follows best practices with beforeEach to reset settings and stub the global location object for each test. The afterEach hook ensures all mocks are reset, preventing test pollution.


33-47: LGTM: Comprehensive test for acceptAll behavior.

This test properly verifies that the acceptAll method:

  1. Calls consentGrantedAll with the correct parameter
  2. Reloads the page
  3. Updates settings with the expected configuration
  4. Calls addGtag to load the tracking script

49-71: LGTM: Thorough test for acceptCustom behavior.

The test correctly verifies custom consent handling, checking that:

  1. The custom consent parameters are properly passed to the consent function
  2. The page is reloaded
  3. Settings are updated correctly
  4. The tracking script is loaded

73-83: Good test coverage for rejectAll behavior.

This test correctly verifies that rejectAll:

  1. Calls consentDeniedAll with the correct parameter
  2. Doesn't reload the page (important difference from accept methods)
  3. Doesn't call addGtag (should not load tracking when rejected)

85-90: LGTM: Proper initial consent state verification.

This test correctly verifies that the initial consent state is false when no cookie is present, and that addGtag is not called when consent is not granted.


92-102: LGTM: Proper cookie-based consent detection.

The test correctly simulates a scenario with an existing GA cookie, verifying that:

  1. The hasConsent value is correctly identified as true
  2. Settings are updated with the expected parameters
  3. The tracking script is loaded automatically
src/core/add-gtag.test.ts (11)

1-16: Good mock setup with proper module imports.

The test file correctly imports dependencies and mocks necessary components. Using vi.importActual allows you to maintain the original implementation while overriding specific functions, which is a good practice.


17-22: Proper test initialization.

The beforeEach hook correctly cleans up the DOM and resets settings before each test, ensuring test isolation.


23-36: LGTM: Basic library load test.

The test correctly verifies that the standard Google Tag Manager script is loaded with the proper URL format, including the tag ID and data layer name.


38-56: Good edge case handling for existing script.

This test correctly verifies the optimization that prevents loading the script multiple times if it's already in the DOM, and ensures the callback is still triggered.


58-74: LGTM: Custom resource URL test.

The test properly verifies that a custom URL can be used for loading the Google Tag Manager script, which is crucial for specialized implementations.


76-89: LGTM: Callback execution test.

This test correctly verifies that the success callback is fired when the script is loaded successfully.


91-106: LGTM: Error handling test.

The test properly verifies error handling by mocking a rejected promise and ensuring the error callback is called.


108-116: LGTM: Configuration initialization test.

This test correctly verifies that the initial configuration is called when the Google Tag Manager script is loaded.


118-135: Good integration test with Vue Router.

The test properly sets up a Vue Router instance and verifies that router tracking is initialized. This ensures that page navigation events will be tracked correctly.


137-141: LGTM: Missing tagId handling test.

The test verifies the important validation that prevents initialization when no tag ID is provided, which would cause errors in Google Analytics.


143-159: Comprehensive script attribute tests.

The tests thoroughly verify that script attributes (preconnect, defer, nonce) are correctly passed to the script injection function, which is important for:

  1. Performance optimization (preconnect)
  2. Non-blocking script loading (defer)
  3. Content Security Policy compliance (nonce)

Also applies to: 161-177, 179-195

src/api/pageview/pageview.ts (3)

1-8: LGTM: Clean type definitions with proper imports.

The type definitions are clear and well-structured, properly leveraging imported types.


39-45: LGTM: Good default handling.

The function properly sets default values for page_location and send_page_view based on either window location or user settings.


47-48: LGTM: Proper event dispatch.

The function correctly uses the query function to send the page view event with the constructed template.

src/core/settings.ts (9)

1-13: LGTM: Clean Vue Router type imports and reexports.

The code properly imports types from Vue Router and re-exports them with clearer names, which improves code readability.


14-23: LGTM: Well-designed type definitions for page tracker parameters.

These type definitions provide flexibility for configuring page tracking, allowing both direct parameters and function-based parameters.


24-63: Excellent PageTracker type with comprehensive JSDoc documentation.

The PageTracker type is well-structured with thorough JSDoc comments explaining each option. This approach provides excellent developer experience and self-documentation.


65-98: LGTM: Well-designed hooks interface with clear documentation.

The hooks interface is well-designed, providing clear lifecycle events for router tracking and script loading. The JSDoc comments clearly explain the purpose and parameters of each hook.


100-123: LGTM: Comprehensive resource configuration options.

The Resource type includes all necessary options for script loading, including URL, preconnect, defer, and nonce support, with clear JSDoc documentation.


125-192: Excellent Settings interface with comprehensive documentation.

The Settings type is well-structured and thoroughly documented. It covers all aspects of Google Analytics configuration, from tag IDs to router tracking to consent modes.


194-195: LGTM: Good type constraint for plugin settings.

The PluginSettings type properly requires the tagId to be present while making other settings optional, which is an excellent practice for API design.


197-202: LGTM: Sensible default settings with readonly modifier.

The default settings object uses the Readonly type to prevent accidental modification, which is a good practice for configuration objects.


204-216: LGTM: Clean settings management functions.

The settings management functions are straightforward and follow good practices:

  1. getSettings returns the current settings
  2. resetSettings restores defaults
  3. updateSettings uses deep merge to properly handle nested objects
package.json (6)

5-5: Confirm version bump.
Bumping to 3.0.0-beta.16 looks intentional and aligns with the upcoming major release changes. Just ensure that existing consumers are adequately informed about any breaking changes in the next stable release.

Could you verify the impact of this version bump on existing consumers, especially those still on 2.x versions?


18-20: Good consolidation of the scripts.
Switching from biome check to biome lint resolves the previously reported missing "check" script. The additional typecheck and test:cov scripts improve project maintenance.

Also applies to: 22-22


39-44: ESM-friendly exports.
Defining an "exports" field for ESM usage is a valuable step. This ensures better module resolution for consumers using ESM.


45-49: Retaining main, module, and dist references.
Maintaining these fields is practical for compatibility with bundlers and older Node versions while also listing "dist" under "files" to ensure a clean package distribution.


56-57: Peer dependencies updated for Vue 3 ecosystem.
Upgrading vue to ^3.5.13 and adding vue-router as a peer dependency ensures clarity for library consumers. Confirm that users on older Vue 3 versions can still integrate or receive a suitable peer dependency warning.

Could you confirm if you have tested or validated compatibility with Vue 3 versions earlier than 3.5.x?


61-62: New development dependencies align with the TS refactor.
Adding @types/node, coverage tools, minification, and DTS plugins is consistent with the migration to a TypeScript + Vite build. Also, providing vue in devDependencies simultaneously with vue in peerDependencies is typical for library development, ensuring local testing.

Also applies to: 66-66, 68-68, 70-70, 72-76

src/types/gtag.ts (4)

1-5: Well-structured union types for configuration.
Combining GtagControlParams, GtagEventParams, GtagConfigParams, and GtagCustomParams in a union is an elegant way to capture varied parameter possibilities without bloating the function signatures.


58-62: [Duplicate] Strengthen return type for the Gtag function.
This replicates a previously raised comment: if gtag("get", ...) or other commands produce data, consider returning that data or a promise rather than strictly void.


94-135: Comprehensive enumeration of event names.
Defining separate event sets for e-commerce (GtagEcommerceEventNames) and general analytics (GtagGeneralEventNames) leads to better type safety, reducing accidental typos or invalid event names.


324-348: Consent mode coverage looks robust.
Providing GtagConsentMode and GtagConsentParams for ad personalization, analytics, and storage strikes a good balance between flexibility and typed constraints. This should align well with future expansions to the consent APIs.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (8)
src/api/event/event.ts (1)

15-17: Consider handling the potential edge case of undefined params.

While this implementation works correctly when params is an object, it could be made more robust by adding a null check before accessing the params.send_to property.

-  if (params.send_to === undefined && additionalAccounts?.length) {
+  if (params?.send_to === undefined && additionalAccounts?.length) {
     params.send_to = groupName;
   }
src/index.test.ts (2)

6-6: Consider using the vi.mock method with inline factory function for more control.

The current mock setup is good, but you could make it more explicit about what's being mocked and potentially add mock implementations.

-vi.mock("./core/add-gtag");
+vi.mock("./core/add-gtag", () => ({
+  default: vi.fn().mockResolvedValue(undefined),
+}));

8-22: Consider adding test cases for error scenarios and custom configurations.

The current tests cover the happy path, but adding tests for error handling and custom configurations would improve test coverage.

Consider adding tests for:

  1. When addGtag throws an error
  2. When custom configuration options are provided
  3. Integration with a router for automatic tracking
src/api/query/query.ts (1)

7-12: Consider restricting the global window interface extension.

The current global window interface extension allows any property to be added with any type, which can lead to type safety issues. Consider narrowing this to only the specific properties needed for this module.

declare global {
  interface Window {
-    // biome-ignore lint/suspicious/noExplicitAny:
-    [key: string]: any | any[];
+    [dataLayerName: string]: any[];
+    [gtagName: string]: (...args: any[]) => void;
  }
}
src/core/add-configuration.ts (1)

32-36: Consider handling custom consent settings.

The current implementation only supports granted/denied for all consent types. Consider adding support for more granular consent settings where users can selectively enable specific consent types.

if (consentMode === "granted") {
  consentGrantedAll();
} else if (consentMode === "denied") {
  consentDeniedAll();
+ } else if (typeof consentMode === "object") {
+   consent("default", consentMode);
}
src/api/consent/consent.ts (1)

8-24: Consider adding support for individual consent preferences.

The current implementation only supports granting or denying all consent types at once. Consider adding helper functions for managing individual consent types for more granular control.

+export function consentUpdate(mode: GtagConsentArg = "default", updates: Partial<GtagConsentParams>) {
+  // Get current consent status or start with all denied
+  const currentConsent: GtagConsentParams = {
+    ad_user_data: "denied",
+    ad_personalization: "denied",
+    ad_storage: "denied",
+    analytics_storage: "denied",
+    // Override with updates
+    ...updates
+  };
+  
+  consent(mode, currentConsent);
+}

+// Helper for analytics only
+export function consentAnalyticsOnly(mode: GtagConsentArg = "default") {
+  consent(mode, {
+    ad_user_data: "denied",
+    ad_personalization: "denied",
+    ad_storage: "denied",
+    analytics_storage: "granted",
+  });
+}
src/core/add-configuration.test.ts (1)

10-153: Consider adding a missing tagId test scenario.

All current tests appear thorough. However, testing the branch where tagId is undefined would ensure that no calls to query occur in that case and further strengthen coverage.

src/core/add-gtag.ts (1)

6-35: Optional SSR safeguard consideration.

This function always attempts to inject or configure gtag on invocation. If server-side rendering fans out requests or calls this function incidentally, you might consider adding an SSR check (e.g., an isServer() guard) before script injection to avoid potential errors. Otherwise, this looks good.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18f991f and aff7440.

📒 Files selected for processing (40)
  • src/api/config/config.test.ts (1 hunks)
  • src/api/config/config.ts (1 hunks)
  • src/api/consent/consent.test.ts (1 hunks)
  • src/api/consent/consent.ts (1 hunks)
  • src/api/custom-map/custom-map.test.ts (1 hunks)
  • src/api/custom-map/custom-map.ts (1 hunks)
  • src/api/ecommerce/ecommerce.test.ts (1 hunks)
  • src/api/ecommerce/ecommerce.ts (1 hunks)
  • src/api/event/event.test.ts (1 hunks)
  • src/api/event/event.ts (1 hunks)
  • src/api/exception/exception.test.ts (1 hunks)
  • src/api/exception/exception.ts (1 hunks)
  • src/api/linker/linker.test.ts (1 hunks)
  • src/api/linker/linker.ts (1 hunks)
  • src/api/opt/opt.test.ts (1 hunks)
  • src/api/opt/opt.ts (1 hunks)
  • src/api/pageview/pageview.test.ts (1 hunks)
  • src/api/pageview/pageview.ts (1 hunks)
  • src/api/query/query.test.ts (1 hunks)
  • src/api/query/query.ts (1 hunks)
  • src/api/screenview/screenview.test.ts (1 hunks)
  • src/api/screenview/screenview.ts (1 hunks)
  • src/api/set/set.test.ts (1 hunks)
  • src/api/set/set.ts (1 hunks)
  • src/api/time/time.test.ts (1 hunks)
  • src/composables/use-consent/use-consent.test.ts (1 hunks)
  • src/composables/use-consent/use-consent.ts (1 hunks)
  • src/core/add-configuration.test.ts (1 hunks)
  • src/core/add-configuration.ts (1 hunks)
  • src/core/add-gtag.test.ts (1 hunks)
  • src/core/add-gtag.ts (1 hunks)
  • src/core/add-router-tracking.test.ts (1 hunks)
  • src/core/add-router-tracking.ts (1 hunks)
  • src/core/settings.ts (1 hunks)
  • src/core/track-route.test.ts (1 hunks)
  • src/core/track-route.ts (1 hunks)
  • src/index.test.ts (1 hunks)
  • src/index.ts (1 hunks)
  • tsconfig.json (1 hunks)
  • vite.config.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (26)
  • src/api/exception/exception.ts
  • src/api/custom-map/custom-map.ts
  • src/api/config/config.test.ts
  • src/api/set/set.ts
  • src/api/linker/linker.test.ts
  • src/api/time/time.test.ts
  • src/core/add-router-tracking.ts
  • src/api/set/set.test.ts
  • src/api/query/query.test.ts
  • tsconfig.json
  • src/core/add-router-tracking.test.ts
  • src/api/linker/linker.ts
  • src/api/exception/exception.test.ts
  • src/api/ecommerce/ecommerce.test.ts
  • src/api/config/config.ts
  • src/api/consent/consent.test.ts
  • src/index.ts
  • src/core/track-route.ts
  • src/composables/use-consent/use-consent.ts
  • src/api/screenview/screenview.ts
  • src/api/opt/opt.test.ts
  • src/core/add-gtag.test.ts
  • src/api/ecommerce/ecommerce.ts
  • src/api/custom-map/custom-map.test.ts
  • src/api/pageview/pageview.ts
  • src/composables/use-consent/use-consent.test.ts
🧰 Additional context used
🧬 Code Definitions (11)
src/index.test.ts (2)
src/index.ts (1)
  • createGtag (16-23)
src/core/add-gtag.ts (1)
  • addGtag (6-35)
src/api/query/query.ts (1)
src/core/settings.ts (1)
  • getSettings (206-208)
src/api/opt/opt.ts (1)
src/core/settings.ts (2)
  • TagId (125-125)
  • getSettings (206-208)
src/api/screenview/screenview.test.ts (3)
src/core/settings.ts (2)
  • resetSettings (210-212)
  • updateSettings (214-216)
src/api/screenview/screenview.ts (1)
  • screenview (11-29)
src/api/query/query.ts (1)
  • query (14-30)
src/core/add-gtag.ts (3)
src/core/settings.ts (1)
  • getSettings (206-208)
src/core/add-configuration.ts (1)
  • addConfiguration (15-59)
src/core/add-router-tracking.ts (1)
  • addRouterTracking (4-24)
src/core/settings.ts (3)
src/api/pageview/pageview.ts (1)
  • Pageview (5-5)
src/api/screenview/screenview.ts (1)
  • Screenview (4-7)
src/api/linker/linker.ts (1)
  • LinkerParams (3-8)
src/api/event/event.ts (2)
src/core/settings.ts (1)
  • getSettings (206-208)
src/api/query/query.ts (1)
  • query (14-30)
src/api/event/event.test.ts (3)
src/api/event/event.ts (1)
  • event (12-20)
src/api/query/query.ts (1)
  • query (14-30)
src/core/settings.ts (1)
  • updateSettings (214-216)
src/core/add-configuration.test.ts (3)
src/core/settings.ts (2)
  • resetSettings (210-212)
  • updateSettings (214-216)
src/core/add-configuration.ts (1)
  • addConfiguration (15-59)
src/api/query/query.ts (1)
  • query (14-30)
src/api/consent/consent.ts (1)
src/api/query/query.ts (1)
  • query (14-30)
src/core/add-configuration.ts (4)
src/core/settings.ts (1)
  • getSettings (206-208)
src/api/consent/consent.ts (2)
  • consentGrantedAll (8-15)
  • consentDeniedAll (17-24)
src/api/linker/linker.ts (1)
  • linker (10-12)
src/api/query/query.ts (1)
  • query (14-30)
🔇 Additional comments (43)
src/api/event/event.ts (1)

1-20: The event function implementation is solid and follows the pattern of other Google Analytics tracking functions.

The function correctly handles the logic for determining when to set the send_to property based on additional accounts, which is important for sending events to the right GA properties. The implementation is concise and focused on a single responsibility.

src/index.test.ts (1)

1-22: Tests adequately cover the plugin initialization and global property installation.

The test suite verifies that:

  1. The plugin installation calls addGtag
  2. The global $gtag property is correctly set to the API

This ensures the Vue plugin works as expected when integrated with applications.

src/core/track-route.test.ts (5)

1-221: Comprehensive test suite covering all aspects of route tracking functionality.

The test suite thoroughly verifies the behavior of the trackRoute function under various configurations:

  • Screen view tracking
  • Page view tracking
  • Route exclusion using different methods
  • Hook callbacks
  • Custom templates

This provides excellent coverage for the tracking functionality.


35-53: Test for screenview tracking is well-structured.

This test correctly verifies that when useScreenview is enabled, the tracking system sends a screen_view event with the appropriate parameters.


55-67: Test for pageview tracking is well-structured.

This test correctly verifies that when screen views are not used (default), the tracking system sends a page_view event with the appropriate parameters including path, location, and title.


69-115: Route exclusion tests cover all supported exclusion methods.

Great job testing all three exclusion methods:

  1. Exclusion by path
  2. Exclusion by route name
  3. Exclusion using a custom function

This ensures routes can be properly excluded from tracking when needed.


157-220: Custom template tests are thorough and cover both object and function templates.

The tests verify that:

  1. Static object templates work for page views
  2. Function-based templates work for page views
  3. Templates work with screen views

This ensures the template system is flexible and works as expected.

src/api/event/event.test.ts (3)

1-58: Good test coverage for the event function with different configurations.

The test suite effectively verifies:

  1. Basic event tracking functionality
  2. Group name handling with additional accounts
  3. Custom group name usage
  4. Preservation of existing send_to property

This ensures the event tracking system works correctly with various configurations.


16-27: The test for group name handling is correct and necessary.

This test verifies an important feature - when multiple tracking accounts are configured, the system correctly adds the group name to the send_to parameter to ensure events are sent to the appropriate account.


43-57: The test for preserving existing send_to property is important.

This test ensures that if a developer explicitly sets a send_to property, the system respects that choice and doesn't override it. This is crucial for allowing manual control when needed.

src/api/query/query.ts (1)

1-5: LGTM! Import and type definitions are well-organized.

The imports and type definitions are properly organized, using path aliases and relevant type imports.

src/core/add-configuration.ts (5)

7-13: Good implementation of mergeDefaults with sensible defaults.

The function correctly preserves user-defined configuration while providing sensible defaults for send_page_view and anonymize_ip.


15-28: Function correctly handles missing tagId case.

The function properly checks if the tagId is present and exits early if it's not, which prevents unnecessary operations.


42-43: LGTM! Proper initialization of Google Analytics.

The code correctly initializes Google Analytics with the standard pattern of first calling "js" with the current date, followed by "config" with the tag ID and configuration.


45-56: LGTM! Proper handling of additional accounts.

The function correctly handles multiple Google Analytics accounts, ensuring each one gets its own configuration call with appropriate settings.


30-31: Safe hook execution pattern.

The code safely checks for hooks before execution with the optional chaining operator, preventing errors if hooks are undefined.

Also applies to: 58-59

src/api/pageview/pageview.test.ts (4)

6-26: LGTM! Proper test setup with router mock.

The test file correctly sets up the testing environment by:

  1. Mocking the query function
  2. Resetting settings before each test
  3. Creating a router with a base path
  4. Waiting for the router to be ready
  5. Navigating to a test route

This provides a consistent testing environment for all test cases.


27-85: LGTM! Comprehensive basic pageview testing.

The tests thoroughly verify basic pageview functionality:

  1. Tracking a page path as a string
  2. Tracking using a route object
  3. Verifying default properties like page_location and page_title
  4. Ensuring send_page_view is true by default

All tests have clear assertions that verify the expected behavior.


87-99: LGTM! Testing custom parameters.

The test correctly verifies that custom parameters can override the defaults, ensuring flexibility in the API.


101-176: LGTM! Thorough testing of pageTracker options.

The tests comprehensively verify that the pageTracker configuration options:

  1. Work with the router reference
  2. Support using the full route path with query params and hash
  3. Support using the router's base path
  4. Allow changing the default send_page_view value

These tests ensure that all configuration options are working as expected.

src/api/consent/consent.ts (3)

1-6: LGTM! Simple and clear consent function.

The consent function correctly passes the consent arguments to the query function, following Google's consent API pattern.


8-15: LGTM! Convenient helper for granting all consent.

The consentGrantedAll function provides a useful shorthand for granting all consent types at once, with a sensible default mode.


17-24: LGTM! Convenient helper for denying all consent.

The consentDeniedAll function provides a useful shorthand for denying all consent types at once, with a sensible default mode.

src/api/opt/opt.ts (1)

1-48: All SSR checks and property assignments look solid.

Your implementation correctly addresses previous feedback by utilizing isServer() checks and deleting the property when enabling tracking. No further issues found.

src/api/screenview/screenview.test.ts (4)

1-7: Good module organization and testing setup.

The imports and mocking are well-structured, properly separating the query functionality for isolated testing.


8-26: Well-structured test environment with clean router setup.

The beforeEach setup correctly initializes the router with predefined routes and properly mocks the router's readiness. This ensures a consistent state for each test case.


28-46: Comprehensive test cases for basic screenview functionality.

These tests correctly verify both string-based and object-based parameter handling, ensuring the screenview function properly tracks events with the expected parameters.


48-84: Good coverage of router integration scenarios.

The nested describe block thoroughly tests pageTracker integration, including:

  1. Application name inclusion
  2. Route name tracking
  3. Fallback to path when no name is available

This ensures the screenview function works correctly with different route configurations.

vite.config.js (5)

3-3: Good addition of TypeScript declaration generation.

The vite-plugin-dts inclusion supports the TypeScript migration by automatically generating declaration files.


20-26: External dependencies properly updated.

The configuration correctly adds vue-router as an external dependency and updates the globals mapping.


30-35: TypeScript integration well configured.

The DTS plugin is properly configured with appropriate options for type generation.


39-42: Enhanced test configuration.

The addition of mockReset and coverage reporting will improve test reliability and provide better insights into test coverage.


14-18:

❓ Verification inconclusive

Verify impact of build format changes on existing users.

The build configuration has been significantly modified:

  1. Entry point changed from JS to TS
  2. Formats reduced to only ES modules (removed UMD and CommonJS)
  3. Static filename instead of format-based naming

These changes align with modern practices but may affect compatibility. Please ensure this is documented in the migration guide mentioned in the PR objectives.


🏁 Script executed:

#!/bin/bash
# Check for any imports of this library using require() which would be affected
rg -g "*.{js,ts,vue}" "require\(['\"]vue-gtag['\"]" || echo "No CommonJS imports found"

Length of output: 118


ACTION: Confirm and Document Build Configuration Changes

The updated build configuration in vite.config.js now:

  • Changes the entry point from a JavaScript file to a TypeScript file.
  • Restricts the output format to only ES modules (removing UMD/CommonJS).
  • Uses a static filename (vue-gtag) rather than a format-based naming convention.

A scan for CommonJS imports (using require('vue-gtag')) returned no results, indicating that within our codebase, there’s no direct dependency on the previously available CommonJS build. However, these changes remain breaking in nature and could affect users consuming the library via UMD or other non-ES methods.

Next Steps:

  • Documentation: Please ensure that these breaking changes are clearly documented in the migration guide referenced in the PR objectives.
  • User Impact: Verify externally (beyond internal scans) that end users are not relying on the removed build formats.
src/core/settings.ts (10)

1-10: Well-organized imports with clear type references.

The imports are properly structured and use explicit types from their respective modules.


11-13: Good practice using type aliases for external dependencies.

Creating aliases for the vue-router types improves code readability and provides flexibility if the underlying implementation changes.


14-23: Clear type definitions for page tracking.

The PageTrackerParams, PageTrackerTemplate, and PageTrackerExclude types are well-defined and provide flexibility for different tracking scenarios.


24-63: Comprehensive PageTracker configuration with excellent documentation.

The PageTracker type provides a robust set of options for configuring page tracking behavior, with clear JSDoc comments explaining each property.


65-98: Well-structured hooks with clear documentation.

The Hooks type provides a comprehensive set of lifecycle events with proper TypeScript function signatures and descriptive comments.


100-123: Resource configuration with security considerations.

The Resource type includes important security features like nonce support for Content Security Policy compliance.


125-192: Comprehensive Settings type with clear documentation.

The Settings type provides a complete configuration surface with well-documented options. The JSDoc comments explain each property clearly.


194-195: Strong type constraints for plugin settings.

The PluginSettings type ensures that the tagId is required while making other settings optional, enforcing proper initialization.


197-204: Sensible default settings with appropriate immutability.

The defaultSettings object uses Readonly to prevent accidental modification, and provides reasonable defaults for required properties.


206-216: Clean settings management API.

The getter and update functions provide a simple and intuitive interface for managing settings, with proper type safety.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (6)
src/core/settings.ts (2)

127-203: Well-structured settings type.
The comprehensive Settings interface covers essential analytics configuration points (resource URLs, dataLayer, hooks, etc.). The doc comments are highly informative. Consider adding JSDoc for any optional fields that might need special handling (e.g., clarifying defaults).


216-229: Efficient state management with getSettings, resetSettings, and updateSettings.
Merging configuration via deepMerge is a powerful approach for complex objects. Ensure that deeply nested arrays or objects are handled as intended (e.g., overwriting vs. merging). A dedicated test can prevent regressions.

src/core/add-gtag.ts (1)

24-35: Robust error handling on script injection.
Catching and propagating the error to hooks?.["script:error"] helps surface failures to the consumer. A fallback or retry might be valuable if script loading is critical.

src/composables/use-consent/use-consent.ts (3)

17-19: Consider a more robust method for consent detection.

The current implementation uses a simple includes() check on document.cookie, which might lead to false positives if another cookie contains the substring "_ga". A more precise approach would check for the exact cookie name.

const hasConsent = ref<boolean>(
-  isServer() ? false : document.cookie.includes(GA_COOKIE_VALUE),
+  isServer() ? false : !!document.cookie
+    .split('; ')
+    .find(row => row.startsWith(`${GA_COOKIE_VALUE}=`))
);

27-29: Add error handling to rejectAll method.

The rejectAll method lacks error handling, which could cause silent failures if the consent operation encounters issues.

const rejectAll = () => {
-  consentDeniedAll("update");
+  try {
+    consentDeniedAll("update");
+  } catch (error) {
+    console.error("Failed to reject all consent:", error);
+  }
};

16-47: Consider adding a way to update consent status without page reloads.

The current implementation doesn't provide a way to update the hasConsent ref when the cookie status changes during the application lifecycle without reloading the page. This could lead to inconsistencies between the actual consent state and what's reflected in the application.

You could add a method to check the current consent status and update the ref accordingly:

export function useConsent(): UseWithConsentReturn {
  const hasConsent = ref<boolean>(
    isServer() ? false : document.cookie.includes(GA_COOKIE_VALUE),
  );

+  // Function to update the current consent status
+  const updateConsentStatus = () => {
+    if (!isServer()) {
+      hasConsent.value = document.cookie.includes(GA_COOKIE_VALUE);
+    }
+  };
+
+  // Consider using this instead of page reloads
+  // and potentially set up a cookie change listener if needed

  // Rest of the implementation...

  return {
    hasConsent,
    acceptAll,
    rejectAll,
    acceptCustom,
+    updateConsentStatus,
  };
}

Additionally, consider implementing a cookie change listener or integrating with a cookie consent manager if your application uses one.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aff7440 and ece6bbb.

📒 Files selected for processing (8)
  • src/composables/use-consent/use-consent.test.ts (1 hunks)
  • src/composables/use-consent/use-consent.ts (1 hunks)
  • src/core/add-gtag.test.ts (1 hunks)
  • src/core/add-gtag.ts (1 hunks)
  • src/core/settings.test.ts (1 hunks)
  • src/core/settings.ts (1 hunks)
  • src/index.test.ts (1 hunks)
  • src/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/core/settings.test.ts
  • src/composables/use-consent/use-consent.test.ts
  • src/core/add-gtag.test.ts
🧰 Additional context used
🧬 Code Definitions (5)
src/core/add-gtag.ts (3)
src/core/settings.ts (1)
  • getSettings (218-220)
src/core/add-configuration.ts (1)
  • addConfiguration (15-59)
src/core/add-router-tracking.ts (1)
  • addRouterTracking (4-24)
src/index.test.ts (3)
src/core/settings.ts (1)
  • resetSettings (222-224)
src/index.ts (2)
  • createGtag (30-38)
  • addGtag (43-43)
src/core/add-gtag.ts (1)
  • addGtag (6-35)
src/composables/use-consent/use-consent.ts (2)
src/core/add-gtag.ts (1)
  • addGtag (6-35)
src/api/consent/consent.ts (3)
  • consentGrantedAll (8-15)
  • consentDeniedAll (17-24)
  • consent (4-6)
src/index.ts (2)
src/core/settings.ts (3)
  • getSettings (218-220)
  • PluginSettings (205-206)
  • updateSettings (226-228)
src/core/add-gtag.ts (1)
  • addGtag (6-35)
src/core/settings.ts (3)
src/api/pageview/pageview.ts (1)
  • Pageview (5-5)
src/api/screenview/screenview.ts (1)
  • Screenview (4-7)
src/api/linker/linker.ts (1)
  • LinkerParams (3-8)
🔇 Additional comments (13)
src/core/settings.ts (2)

1-9: Imports are clear and organized.
All imported types and utilities appear relevant and properly referenced, ensuring good modular separation.


208-215: Default settings are concise and safe.
Storing a small set of essential defaults in defaultSettings helps ensure stable behavior in the absence of user overrides. Good practice!

src/index.test.ts (3)

9-16: Clear testing of plugin installation.
Ensuring addGtag is called upon plugin creation (when initMode is not "manual") verifies correct default behavior. The test is concise and informative.


17-25: Manual installation test is comprehensive.
The sequence checks that addGtag isn't invoked initially, then called explicitly via initialize(), which matches the initMode: "manual" specification.


27-34: Verifying $gtag global property.
The test ensures that $gtag is properly attached to app.config.globalProperties, confirming the plugin's integration is as expected.

src/core/add-gtag.ts (2)

6-11: Graceful handling of missing tag ID.
Early return when tagId is absent prevents unnecessary script injection or router tracking, which is a good defensive measure.


19-23: Double-injection prevention logic is clear.
By checking hasGtag() before injecting the script, you avoid reloading or overwriting an already loaded gtag instance. This is a good practice for idempotency.

src/index.ts (3)

1-2: Re-exporting API and the addGtag function enhances discoverability.
By centralizing exports, you make it easier for consumers to import what they need. Nicely organized.


20-28: Conditional gtag initialization with handleGtag().
initMode === "manual" check ensures the library does not auto-load if the user explicitly wants to invoke addGtag. This is a nifty separation of concerns.


30-38: [Duplicate suggestion about awaiting addGtag]

-  handleGtag();
+  void handleGtag(); // or await handleGtag() in an async context

If subsequent operations depend on gtag being fully loaded, consider awaiting for reliability. Since this mirrors a prior reviewer comment, marking as duplicate.

src/composables/use-consent/use-consent.ts (3)

21-25: Avoid page reloads and add error handling.

Reloading the page in the acceptAll method disrupts the user experience and loses application state. Additionally, there's no error handling for the asynchronous operations.

const acceptAll = async () => {
-  await addGtag();
-  consentGrantedAll("update");
-  window.location.reload();
+  try {
+    await addGtag();
+    consentGrantedAll("update");
+    // If reload is absolutely necessary, add a comment explaining why
+    // window.location.reload();
+  } catch (error) {
+    console.error("Failed to accept all consent:", error);
+  }
};

31-35: Avoid page reloads and add error handling for acceptCustom.

Similar to acceptAll, the acceptCustom method unnecessarily reloads the page and lacks error handling.

const acceptCustom = async (params: GtagConsentParams) => {
-  await addGtag();
-  consent("update", params);
-  window.location.reload();
+  try {
+    await addGtag();
+    consent("update", params);
+    // If reload is absolutely necessary, add a comment explaining why
+    // window.location.reload();
+  } catch (error) {
+    console.error("Failed to accept custom consent:", error);
+  }
};

37-39: Add error handling around initial addGtag call.

The initial call to addGtag when hasConsent.value is true lacks error handling, which could lead to unhandled promise rejections.

if (hasConsent.value) {
-  addGtag();
+  // Initialize Google Analytics if consent is already granted
+  addGtag().catch(error => {
+    console.error("Failed to initialize Google Analytics:", error);
+  });
}

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
package.json (1)

45-47: Potential Redundancy with Legacy Fields.
The file still includes "main", "module", and "types" fields even though an "exports" field is defined. While these may be kept for backward compatibility, it might be useful to add an inline comment clarifying their purpose.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ece6bbb and 484f652.

📒 Files selected for processing (1)
  • package.json (3 hunks)
🔇 Additional comments (14)
package.json (14)

5-5: Version Bump Update.
The version field was updated to "3.0.0-beta.19", which correctly reflects the major release bump per the PR objectives.


18-18: Updated Lint Command.
The "lint" script is now set to "biome lint", replacing an older check command. This better aligns with the current tooling.


19-19: Enhanced Build Process Validation.
The "build" script now runs "tsc --noEmit && vite build", ensuring that type errors are caught before executing the build step. This extra check improves build robustness.


20-20: Addition of Dedicated Type Check.
Introducing the "typecheck" script ("tsc --noEmit") provides an isolated way to verify type integrity. This is a valuable addition for maintaining type safety.


22-22: New Test Coverage Script.
The "test:cov" script ("vitest run --coverage") facilitates runtime test coverage analysis, supporting ongoing quality assurance efforts.


24-24: Revised Prepublish Process.
The "prepublishOnly" script has been updated to:

"npm run lint && npm run test:once && npm run build"

This change replaces the earlier use of "yarn check" with "npm run lint", ensuring consistency with the rest of the npm-based scripts. Please verify that switching to npm here is intentional.


39-44: Modernized Exports Configuration.
The introduction of the "exports" field now explicitly defines the import path and types file:

"exports": {
  ".": {
    "types": "./dist/vue-gtag.d.ts",
    "import": "./dist/vue-gtag.js"
  }
}

This modern approach improves module resolution for consumers.


49-49: Streamlined Files Inclusion.
Limiting the "files" array to just "dist" ensures that only the necessary build artifacts are published, reducing package bloat.


56-57: Updated Peer Dependencies.
The peer dependencies now require:

  • "vue": "^3.5.13"
  • "vue-router": "^4.5.0"

These updates align with the latest framework versions and intended compatibility improvements.


61-62: Enhanced Dev Dependency Versions.
Adding "@types/node": "^22.13.5" and "@vitest/coverage-v8": "^3.0.5" ensures that the development environment is up-to-date and leverages current type definitions and test coverage tools.


66-66: Inclusion of Terser for Code Minification.
The addition of "terser": "^5.39.0" will help optimize and minify the code during builds. Confirm that this version meets your performance and compatibility requirements.


68-68: Introducing Vite Plugin for DTS Generation.
The new dev dependency "vite-plugin-dts": "^4.5.0" enhances type declaration generation, improving the development experience and maintaining robust type information.


70-70: Aligned Vue Version in Dev Dependencies.
Updating "vue" in the dev dependencies to "^3.5.13" aligns it with the peer dependency, avoiding potential version conflicts during development.


73-77: Optional Peer Dependency Configuration.
The newly added "peerDependenciesMeta" field marks "vue-router" as optional:

"peerDependenciesMeta": {
  "vue-router": {
    "optional": true
  }
}

This configuration provides flexibility for users who may not need routing capabilities.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (12)
src/core/add-router-tracking/add-router-tracking.ts (1)

4-24: Good implementation of router tracking with proper flow control.

The implementation correctly handles router tracking by:

  1. Checking if router is available
  2. Waiting for router to be ready before tracking the initial route
  3. Setting up tracking for future route changes
  4. Avoiding duplicate tracking when path hasn't changed

Consider adding JSDoc documentation to explain the purpose and behavior of this function, particularly noting that it automatically integrates with Vue Router:

+/**
+ * Sets up automatic route tracking with Vue Router.
+ * 
+ * This function retrieves router configuration from settings,
+ * tracks the initial route, and sets up tracking for all subsequent
+ * route changes, while avoiding duplicate tracking for the same path.
+ * 
+ * @returns A promise that resolves when router tracking is set up
+ */
 export async function addRouterTracking(): Promise<void> {
src/core/settings/settings.test.ts (1)

3-27: Test coverage is good but could benefit from better test isolation.

The tests correctly verify both the default configuration and the update functionality.

Consider resetting the settings after the second test to ensure test isolation and prevent potential side effects in other tests:

 it("should update the default configuration", () => {
   updateSettings({
     tagId: "UA-1234567",
     gtagName: "foo",
   });

   expect(getSettings()).toEqual(
     expect.objectContaining({
       tagId: "UA-1234567",
       gtagName: "foo",
     }),
   );
+  
+  // Reset settings after test to ensure isolation
+  resetSettings();
 });
src/core/add-configuration/add-configuration.ts (1)

15-59: Comprehensive configuration implementation but missing return type and error handling.

The function correctly handles all aspects of configuration including consent mode, linker options, and additional accounts.

Consider these improvements:

  1. Add a return type annotation to the function
  2. Add JSDoc documentation to explain the function's purpose
  3. Implement error handling for potential failures during configuration
/**
 * Initializes and configures Google Analytics tracking based on current settings.
 * 
 * Sets up consent mode, linker for cross-domain tracking, and configures 
 * both the main tag ID and any additional accounts.
 * 
 * @throws Potential errors from the underlying query functions are not handled
 * @returns void
 */
-export function addConfiguration() {
+export function addConfiguration(): void {

Additionally, consider wrapping the query calls in try/catch blocks to handle potential errors:

-  query("js", new Date());
-  query("config", tagId, mergeDefaults(config));
+  try {
+    query("js", new Date());
+    query("config", tagId, mergeDefaults(config));
+  } catch (error) {
+    console.error("Failed to initialize Google Analytics:", error);
+    hooks?.["config:init:error"]?.(error);
+  }
src/core/settings/settings.ts (1)

12-24: Well-structured settings module with appropriate immutability patterns.

The implementation correctly handles settings management with getters, setters, and reset functionality. The use of deep merge for updates is appropriate for handling nested configuration objects.

Consider adding JSDoc documentation to clarify the purpose of each function:

+/**
+ * Returns the current settings configuration.
+ * @returns The current settings object
+ */
 export function getSettings(): Settings {
   return settings;
 }

+/**
+ * Resets all settings to their default values.
+ */
 export function resetSettings(): void {
   settings = { ...defaultSettings };
 }

+/**
+ * Updates the current settings by deeply merging the provided configuration.
+ * @param configParams Partial settings to merge with current configuration
+ */
 export function updateSettings(configParams: Partial<Settings>): void {
   settings = deepMerge(settings, configParams);
 }
src/core/add-gtag/add-gtag.ts (3)

20-22: The function should maintain its Promise chain correctly

When gtag is already loaded, the function returns early but still needs to return a resolved Promise to maintain consistent async behavior.

  if (hasGtag()) {
    hooks?.["script:loaded"]?.();
-    return;
+    return Promise.resolve();
  }

31-31: Consider awaiting hooks if they might be async

The script loaded hook might be asynchronous, but it's not being awaited. If the hook performs asynchronous operations, they might not complete before the function returns.

-    hooks?.["script:loaded"]?.();
+    await hooks?.["script:loaded"]?.();

33-33: Consider awaiting error hooks if they might be async

Similar to the loaded hook, the error hook might be asynchronous, but it's not being awaited.

-    hooks?.["script:error"]?.(error);
+    await hooks?.["script:error"]?.(error);
src/core/track-route/track-route.ts (4)

20-22: Simplify the optional chaining in array methods

The optional chaining on pageTracker.exclude?.some() is redundant since we've already checked for !pageTracker?.exclude on line 12.

-  return pageTracker.exclude?.some(({ name, path } = {}) => {
+  return pageTracker.exclude.some(({ name, path } = {}) => {
    return (name && name === route.name) || (path && path === route.path);
  });

32-32: Consider awaiting hooks if they might be async

If the router:track:before hook might perform async operations, you should await it to ensure they complete before continuing with tracking.

-  hooks?.["router:track:before"]?.(route);
+  await hooks?.["router:track:before"]?.(route);

55-55: Consider awaiting hooks if they might be async

Similar to the before hook, if the router:track:after hook might perform async operations, you should await it.

-  hooks?.["router:track:after"]?.(route);
+  await hooks?.["router:track:after"]?.(route);

43-53: Consider using type guards for cleaner conditional logic

The current approach of using in operator for type checking works, but type guards could make the code more readable and type-safe.

You could define type guards:

function isScreenviewParams(params: any): params is Screenview {
  return params && 'screen_name' in params;
}

function isPageviewParams(params: any): params is Pageview {
  return params && 'page_path' in params;
}

Then use them in your conditional logic:

  if (pageTracker?.useScreenview) {
-    const screenviewParams =
-      template && "screen_name" in template ? template : route;
+    const screenviewParams = template && isScreenviewParams(template) 
+      ? template 
+      : route;

    screenview(screenviewParams);
  } else {
-    const pageviewParams =
-      template && "page_path" in template ? template : route;
+    const pageviewParams = template && isPageviewParams(template) 
+      ? template 
+      : route;

    pageview(pageviewParams);
  }
src/core/add-configuration/add-configuration.test.ts (1)

115-150: Consider additional scenarios for partial consent modes.
The current tests thoroughly cover the “granted” and “denied” modes. If future requirements include partially granted consent (e.g., analytics only), consider adding tests to cover those scenarios.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 484f652 and e725ec5.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (16)
  • src/core/add-configuration/add-configuration.test.ts (1 hunks)
  • src/core/add-configuration/add-configuration.ts (1 hunks)
  • src/core/add-configuration/index.ts (1 hunks)
  • src/core/add-gtag/add-gtag.test.ts (1 hunks)
  • src/core/add-gtag/add-gtag.ts (1 hunks)
  • src/core/add-gtag/index.ts (1 hunks)
  • src/core/add-router-tracking/add-router-tracking.test.ts (1 hunks)
  • src/core/add-router-tracking/add-router-tracking.ts (1 hunks)
  • src/core/add-router-tracking/index.ts (1 hunks)
  • src/core/settings/index.ts (1 hunks)
  • src/core/settings/settings.test.ts (1 hunks)
  • src/core/settings/settings.ts (1 hunks)
  • src/core/settings/types.ts (1 hunks)
  • src/core/track-route/index.ts (1 hunks)
  • src/core/track-route/track-route.test.ts (1 hunks)
  • src/core/track-route/track-route.ts (1 hunks)
✅ Files skipped from review due to trivial changes (5)
  • src/core/add-gtag/index.ts
  • src/core/track-route/index.ts
  • src/core/add-router-tracking/index.ts
  • src/core/add-configuration/index.ts
  • src/core/settings/index.ts
🧰 Additional context used
🧬 Code Definitions (8)
src/core/add-router-tracking/add-router-tracking.ts (2)
src/core/settings/settings.ts (1)
  • getSettings (14-16)
src/core/track-route/track-route.ts (1)
  • trackRoute (25-56)
src/core/add-configuration/add-configuration.ts (1)
src/core/settings/settings.ts (1)
  • getSettings (14-16)
src/core/add-gtag/add-gtag.ts (3)
src/core/settings/settings.ts (1)
  • getSettings (14-16)
src/core/add-configuration/add-configuration.ts (1)
  • addConfiguration (15-59)
src/core/add-router-tracking/add-router-tracking.ts (1)
  • addRouterTracking (4-24)
src/core/settings/settings.test.ts (1)
src/core/settings/settings.ts (2)
  • getSettings (14-16)
  • updateSettings (22-24)
src/core/add-configuration/add-configuration.test.ts (2)
src/core/settings/settings.ts (2)
  • resetSettings (18-20)
  • updateSettings (22-24)
src/core/add-configuration/add-configuration.ts (1)
  • addConfiguration (15-59)
src/core/settings/settings.ts (1)
src/core/settings/types.ts (1)
  • Settings (126-202)
src/core/track-route/track-route.ts (2)
src/core/settings/types.ts (2)
  • Route (11-11)
  • PageTrackerParams (13-13)
src/core/settings/settings.ts (1)
  • getSettings (14-16)
src/core/add-gtag/add-gtag.test.ts (4)
src/core/settings/settings.ts (2)
  • resetSettings (18-20)
  • updateSettings (22-24)
src/core/add-gtag/add-gtag.ts (1)
  • addGtag (6-35)
src/core/add-configuration/add-configuration.ts (1)
  • addConfiguration (15-59)
src/core/add-router-tracking/add-router-tracking.ts (1)
  • addRouterTracking (4-24)
🔇 Additional comments (12)
src/core/add-configuration/add-configuration.ts (1)

7-13: Good pattern for setting sensible defaults while preserving custom configuration.

The mergeDefaults function correctly provides default values while allowing custom configuration to override them.

src/core/settings/settings.ts (1)

4-10: Good practice using a readonly constant for default settings.

The default settings are properly defined as a readonly constant with sensible default values.

src/core/add-router-tracking/add-router-tracking.test.ts (1)

1-92: Test coverage looks comprehensive and well-structured

The test suite effectively covers the three main scenarios for router tracking:

  1. Initial tracking when the router is ready
  2. Tracking after a route change
  3. Skipping tracking for identical paths

The mocking approach and assertions are clear and verify the expected behavior.

src/core/add-gtag/add-gtag.test.ts (1)

1-196: Test suite is thorough and comprehensive

The test suite covers all major scenarios including:

  • Basic library download
  • Avoiding duplicate downloads
  • Custom resource handling
  • Callback execution
  • Configuration initialization
  • Router tracking initialization
  • Missing tag ID handling
  • Script attribute verification (preconnect, defer, nonce)
src/core/add-configuration/add-configuration.test.ts (2)

1-9: Great setup for the test suite.
All the initial imports, mocking of query, and date handling with mockdate are well-structured and straightforward. No changes needed.


10-112: Solid coverage for configuration behaviors and hooks.
These tests comprehensively validate multiple scenarios—initial configuration, additional accounts, and linker parameters—while also verifying hooks for config:init:before and config:init:after. They look well-structured and logically sound.

src/core/track-route/track-route.test.ts (4)

1-34: Good test environment setup.
The imports, router initialization, and mocking of query create a clear foundation for verifying route tracking.


35-67: Thorough screenview and pageview coverage.
These tests demonstrate correct event parameters for both screenview and pageview tracking. The logic appears accurate.


69-115: Robust route exclusion tests.
Exclusions based on path, name, and custom functions work as intended. No issues identified.


117-220: Flexible handling of callbacks and custom templates.
Tracking hooks (before, after) and custom templates (both static and function-based) are thoroughly covered. This ensures a wide range of tracking customizations.

src/core/settings/types.ts (2)

1-97: Well-defined type structures for page tracking and hooks.
The PageTracker and Hooks types clearly capture the configurable parameters and user-defined callbacks. All fields appear logically grouped.


99-206: Comprehensive settings definition.
The Settings and PluginSettings types provide a robust backbone for the analytics configuration, including multi-account setups, consent modes, and plugin extensibility. This is clean and maintainable.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/core/create-gtag/create-gtag.test.ts (1)

1-30: Good test coverage, consider adding more edge cases

The test file has a good structure and basic coverage for the main functionality of createGtag. The three test cases validate the core behaviors: initialization, Vue integration, and manual mode skipping initialization.

Consider adding these additional test cases for more comprehensive coverage:

  • Error handling when tagId is missing or invalid
  • Testing with various configuration options (e.g., different initMode values)
  • Testing the behavior when called multiple times (idempotence)
it("should not initialize gtag when tagId is missing", () => {
  createGtag({});
  expect(addGtag).not.toHaveBeenCalled();
});

it("should support different initMode values", () => {
  createGtag({ tagId: "UA-123456789", initMode: "onLoad" });
  expect(addGtag).toHaveBeenCalled();
});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e725ec5 and 838890c.

📒 Files selected for processing (6)
  • src/core/add-gtag/add-gtag.ts (1 hunks)
  • src/core/create-gtag/create-gtag.test.ts (1 hunks)
  • src/core/create-gtag/create-gtag.ts (1 hunks)
  • src/core/create-gtag/index.ts (1 hunks)
  • src/core/index.ts (1 hunks)
  • src/index.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • src/core/create-gtag/index.ts
  • src/core/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/index.ts
  • src/core/add-gtag/add-gtag.ts
🧰 Additional context used
🧬 Code Definitions (1)
src/core/create-gtag/create-gtag.ts (1)
src/core/add-gtag/add-gtag.ts (1)
  • addGtag (10-39)

@MatteoGabriele MatteoGabriele merged commit 53323d8 into master Mar 28, 2025
2 checks passed
@MatteoGabriele MatteoGabriele deleted the feat/v3 branch March 28, 2025 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

2 participants