feat(types-google-analytics): unified GA4 + GTM dataLayer types#298
feat(types-google-analytics): unified GA4 + GTM dataLayer types#298stephansama wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 4e92a5b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a new TypeScript types-only package ChangesNew types-google-analytics Package
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
@stephansama/ai-commit-msg
@stephansama/alfred-kaomoji
@stephansama/astro-iconify-svgmap
@stephansama/auto-readme
@stephansama/catppuccin-jsonresume-theme
@stephansama/catppuccin-opml
@stephansama/catppuccin-rss
@stephansama/catppuccin-typedoc
@stephansama/catppuccin-xsl
@stephansama/eslint-config
create-stephansama-example
@stephansama/find-makefile-targets
@stephansama/github-env
@stephansama/multipublish
@stephansama/pnpm-hooks
@stephansama/prettier-plugin-handlebars
@stephansama/remark-asciinema
@stephansama/single-file
@stephansama/svelte-social-share-links
@stephansama/typed-env
@stephansama/typed-events
@stephansama/typed-nocodb-api
@stephansama/typed-templates
@stephansama/types-github-action-env
@stephansama/types-google-analytics
@stephansama/types-lhci
commit: |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces @stephansama/google-analytics-types, a new types-only package providing TypeScript definitions for Google Tag Manager's dataLayer and GA4's gtag events. The package includes interfaces for recommended GA4 events, ecommerce items, and global window augmentation. Feedback from the review highlights that the DataLayerEvent union is currently too broad, which bypasses strict validation for specific events. Additionally, the Node.js engine requirement needs to be adjusted to a supported version, and the items field in the purchase event should be marked as required to align with GA4 documentation.
| export type DataLayerEvent = | ||
| | (Record<string, unknown> & { event: string }) | ||
| | { | ||
| [K in GA4EventName]: GA4EventParamsMap[K] & { event: K }; | ||
| }[GA4EventName]; |
There was a problem hiding this comment.
The current definition of DataLayerEvent includes a catch-all (Record<string, unknown> & { event: string }). Because this is a union, TypeScript will match any object with an event property against this broad type, effectively disabling validation for the recommended GA4 events (e.g., a purchase event missing required fields will not trigger an error).
To ensure strict typing and validation, the catch-all should be removed. Users can still support custom events by extending the GA4EventParamsMap interface via module augmentation, as noted in your documentation.
export type DataLayerEvent = {
[K in GA4EventName]: GA4EventParamsMap[K] & { event: K };
}[GA4EventName];| "tsdown": "catalog:" | ||
| }, | ||
| "engines": { | ||
| "node": ">=24" |
There was a problem hiding this comment.
The Node.js engine requirement is set to >=24, but Node.js 24 has not been released yet (Node 22 is the current LTS and Node 23 is the current stable). This will prevent the package from being installed on most current environments. You likely intended to target >=20 or >=22.
| "node": ">=24" | |
| "node": ">=22" |
| export interface GA4PurchaseEventParams { | ||
| coupon?: string; | ||
| currency: string; | ||
| items?: GA4Item[]; |
There was a problem hiding this comment.
According to the GA4 recommended events reference, the items parameter is required for the purchase event.
| items?: GA4Item[]; | |
| items: GA4Item[]; |
stephansama
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/types-google-analytics/package.json`:
- Around line 47-49: The package.json currently pins the "engines" field to
"node": ">=24", which is unnecessarily restrictive for a types-only package (see
"sideEffects": false); update the package.json by either removing the "engines"
entry entirely or relaxing it to a broader LTS range such as "node": ">=18" so
consumers on Node 18/20/22 can install the package; locate and edit the
"engines" object/string in the package.json to apply this change.
In `@core/types-google-analytics/README.md`:
- Line 1: Update the README title in README.md to use the correct package name:
replace the current "`@stephansama/google-analytics-types`" heading with
"`@stephansama/types-google-analytics`" so it matches the package and directory
naming (verify the top-line H1 string and any other occurrences in the file).
In `@core/types-google-analytics/src/index.test.ts`:
- Line 14: The test suite description string in the unit test (the describe(...)
call in index.test.ts) uses the wrong package name; change the describe label
from "`@stephansama/google-analytics-types`" to the correct package name
"`@stephansama/types-google-analytics`" so it matches the directory and PR
metadata (update the string inside the describe call).
- Around line 25-29: Replace the loose structural check on
GA4PurchaseEventParameters with an assertion that the actual gtag invocation was
type-checked: assert that the third parameter type of the gtag call (e.g.
Parameters<typeof gtag>[2]) matches the expected shape (properties currency:
string, transaction_id: string, value: number). Update the test to use
expectTypeOf on Parameters<typeof gtag>[2] (or the appropriate call signature
type) against the GA4PurchaseEventParameters shape so the overload resolution
for the gtag call is validated directly.
- Around line 6-7: Remove the unnecessary type aliases in the import: stop
importing GA4EventParametersMap and GA4PurchaseEventParameters as aliases and
import the actual exported names GA4EventParamsMap and GA4PurchaseEventParams
directly; then replace any uses of the alias identifiers (GA4EventParametersMap,
GA4PurchaseEventParameters) in the test file with the real names
GA4EventParamsMap and GA4PurchaseEventParams (these occur where the aliases were
referenced later in the file).
In `@core/types-google-analytics/src/index.ts`:
- Around line 167-171: The loose custom-event overload for the Gtag type
currently allows any string (including known GA4EventName) and should be
narrowed so known event names are forced to use the strongly-typed overload;
update the second overload signature for Gtag (the one with command: "event") to
use name: Exclude<string, GA4EventName> so that known GA4EventName values bind
to the strict first overload that uses GA4EventParamsMap, preventing accidental
bypass of the typed params checks.
- Around line 21-25: DataLayerEvent's first union branch (Record<string,
unknown> & { event: string }) is too permissive and lets known GA4 event names
(e.g. "purchase") match that branch and bypass the stricter
GA4PurchaseEventParams; change that branch so its event property excludes
GA4EventName (e.g. { event: Exclude<string, GA4EventName> } or Record<string,
unknown> & { event: Exclude<string, GA4EventName> }) so that known GA4EventName
values are forced to match the GA4EventParamsMap lookup branch (refer to
DataLayerEvent, GA4EventName, GA4EventParamsMap).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc1dfb74-7d4b-4e42-a3cd-f4468c4cc140
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
README.mdcore/types-google-analytics/README.mdcore/types-google-analytics/package.jsoncore/types-google-analytics/src/index.test.tscore/types-google-analytics/src/index.tscore/types-google-analytics/tsconfig.build.jsoncore/types-google-analytics/tsconfig.json
| "engines": { | ||
| "node": ">=24" | ||
| }, |
There was a problem hiding this comment.
Unnecessarily restrictive Node engine requirement for a types-only package.
Requiring Node >=24 will block adoption by projects using Node 18 LTS, Node 20 LTS, or Node 22 LTS. Since this is a types-only package with no runtime dependencies and sideEffects: false, there is no technical reason to require such a recent Node version. TypeScript type definitions are consumed at compile time and do not depend on Node runtime features.
📦 Recommended fix to support LTS Node versions
"engines": {
- "node": ">=24"
+ "node": ">=18"
},Alternatively, if you have a specific reason for requiring Node 24+, consider omitting the engines field entirely for maximum compatibility.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "engines": { | |
| "node": ">=24" | |
| }, | |
| "engines": { | |
| "node": ">=18" | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/types-google-analytics/package.json` around lines 47 - 49, The
package.json currently pins the "engines" field to "node": ">=24", which is
unnecessarily restrictive for a types-only package (see "sideEffects": false);
update the package.json by either removing the "engines" entry entirely or
relaxing it to a broader LTS range such as "node": ">=18" so consumers on Node
18/20/22 can install the package; locate and edit the "engines" object/string in
the package.json to apply this change.
| @@ -0,0 +1,36 @@ | |||
| # @stephansama/google-analytics-types | |||
There was a problem hiding this comment.
Fix package name to match directory structure.
The README title shows @stephansama/google-analytics-types, but the PR summary and directory path (core/types-google-analytics/) indicate the correct package name is @stephansama/types-google-analytics. The word order is reversed.
🐛 Correct the package name
-# `@stephansama/google-analytics-types`
+# `@stephansama/types-google-analytics`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # @stephansama/google-analytics-types | |
| # `@stephansama/types-google-analytics` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/types-google-analytics/README.md` at line 1, Update the README title in
README.md to use the correct package name: replace the current
"`@stephansama/google-analytics-types`" heading with
"`@stephansama/types-google-analytics`" so it matches the package and directory
naming (verify the top-line H1 string and any other occurrences in the file).
| GA4EventParamsMap as GA4EventParametersMap, | ||
| GA4PurchaseEventParams as GA4PurchaseEventParameters, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Remove unnecessary type aliases.
The aliases GA4EventParametersMap and GA4PurchaseEventParameters are inconsistent with the actual exported type names (GA4EventParamsMap and GA4PurchaseEventParams). This renaming from "Params" to "Parameters" creates confusion without adding value.
♻️ Simplify imports by removing aliases
import type {
DataLayerEvent,
GA4EventName,
- GA4EventParamsMap as GA4EventParametersMap,
- GA4PurchaseEventParams as GA4PurchaseEventParameters,
+ GA4EventParamsMap,
+ GA4PurchaseEventParams,
Gtag,
} from "./index";Then update line 25 and line 48:
- expectTypeOf<GA4PurchaseEventParameters>().toMatchTypeOf<{
+ expectTypeOf<GA4PurchaseEventParams>().toMatchTypeOf<{- expectTypeOf<GA4EventName>().toEqualTypeOf<
- keyof GA4EventParametersMap
- >();
+ expectTypeOf<GA4EventName>().toEqualTypeOf<keyof GA4EventParamsMap>();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/types-google-analytics/src/index.test.ts` around lines 6 - 7, Remove the
unnecessary type aliases in the import: stop importing GA4EventParametersMap and
GA4PurchaseEventParameters as aliases and import the actual exported names
GA4EventParamsMap and GA4PurchaseEventParams directly; then replace any uses of
the alias identifiers (GA4EventParametersMap, GA4PurchaseEventParameters) in the
test file with the real names GA4EventParamsMap and GA4PurchaseEventParams
(these occur where the aliases were referenced later in the file).
| // These are type-level assertions — the runtime body of each `it` is empty | ||
| // because the value of the test is in `expectTypeOf` at compile time. | ||
|
|
||
| describe("@stephansama/google-analytics-types", () => { |
There was a problem hiding this comment.
Fix package name to match directory structure.
The test suite describes @stephansama/google-analytics-types, but the PR summary and directory path (core/types-google-analytics/) indicate the package name should be @stephansama/types-google-analytics. The word order is reversed.
🐛 Correct the package name
-describe("`@stephansama/google-analytics-types`", () => {
+describe("`@stephansama/types-google-analytics`", () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe("@stephansama/google-analytics-types", () => { | |
| describe("`@stephansama/types-google-analytics`", () => { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/types-google-analytics/src/index.test.ts` at line 14, The test suite
description string in the unit test (the describe(...) call in index.test.ts)
uses the wrong package name; change the describe label from
"`@stephansama/google-analytics-types`" to the correct package name
"`@stephansama/types-google-analytics`" so it matches the directory and PR
metadata (update the string inside the describe call).
| expectTypeOf<GA4PurchaseEventParameters>().toMatchTypeOf<{ | ||
| currency: string; | ||
| transaction_id: string; | ||
| value: number; | ||
| }>(); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Strengthen type assertion to verify gtag call enforcement.
The current assertion only verifies that the GA4PurchaseEventParams type definition includes certain properties, but doesn't confirm that the gtag call on lines 19-24 was actually type-checked against those requirements. Consider asserting on the call's parameter types directly.
💡 Example of a stronger assertion
- expectTypeOf<GA4PurchaseEventParams>().toMatchTypeOf<{
- currency: string;
- transaction_id: string;
- value: number;
- }>();
+ expectTypeOf(gtag).parameter(2).toMatchTypeOf<GA4PurchaseEventParams>();This approach verifies that the third parameter of the gtag call matches the expected type, confirming the overload resolution works correctly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/types-google-analytics/src/index.test.ts` around lines 25 - 29, Replace
the loose structural check on GA4PurchaseEventParameters with an assertion that
the actual gtag invocation was type-checked: assert that the third parameter
type of the gtag call (e.g. Parameters<typeof gtag>[2]) matches the expected
shape (properties currency: string, transaction_id: string, value: number).
Update the test to use expectTypeOf on Parameters<typeof gtag>[2] (or the
appropriate call signature type) against the GA4PurchaseEventParameters shape so
the overload resolution for the gtag call is validated directly.
| export type DataLayerEvent = | ||
| | (Record<string, unknown> & { event: string }) | ||
| | { | ||
| [K in GA4EventName]: GA4EventParamsMap[K] & { event: K }; | ||
| }[GA4EventName]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify broad branch currently allows known event names.
rg -n -C2 'export type DataLayerEvent|event:\s*string|Exclude<string,\s*GA4EventName>' core/types-google-analytics/src/index.tsRepository: stephansama/packages
Length of output: 307
🏁 Script executed:
#!/bin/bash
# Find GA4EventName and GA4EventParamsMap definitions
rg -n 'GA4EventName|GA4EventParamsMap' core/types-google-analytics/src/index.ts | head -30Repository: stephansama/packages
Length of output: 323
🏁 Script executed:
#!/bin/bash
# Get a broader view of the file to understand the type structure
wc -l core/types-google-analytics/src/index.tsRepository: stephansama/packages
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Read the GA4EventParamsMap definition and context around DataLayerEvent
sed -n '45,160p' core/types-google-analytics/src/index.tsRepository: stephansama/packages
Length of output: 3542
🏁 Script executed:
#!/bin/bash
# Check for test files or examples using DataLayerEvent
find core/types-google-analytics -name "*.test.*" -o -name "*.spec.*" -o -name "*.example.*" 2>/dev/null | head -10Repository: stephansama/packages
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Search for any usage or imports of DataLayerEvent
rg -n 'DataLayerEvent' --type tsRepository: stephansama/packages
Length of output: 375
🏁 Script executed:
#!/bin/bash
# Read the test file to understand type validation
cat -n core/types-google-analytics/src/index.test.tsRepository: stephansama/packages
Length of output: 1804
DataLayerEvent currently accepts invalid payloads for known GA4 events.
Line 22 makes the union too broad with event: string, allowing { event: "purchase" } to match the first branch and bypass the typed GA4PurchaseEventParams requirements (currency, transaction_id, value are all required).
Proposed fix
export type DataLayerEvent =
- | (Record<string, unknown> & { event: string })
+ | (Record<string, unknown> & { event: Exclude<string, GA4EventName> })
| {
[K in GA4EventName]: GA4EventParamsMap[K] & { event: K };
}[GA4EventName];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/types-google-analytics/src/index.ts` around lines 21 - 25,
DataLayerEvent's first union branch (Record<string, unknown> & { event: string
}) is too permissive and lets known GA4 event names (e.g. "purchase") match that
branch and bypass the stricter GA4PurchaseEventParams; change that branch so its
event property excludes GA4EventName (e.g. { event: Exclude<string,
GA4EventName> } or Record<string, unknown> & { event: Exclude<string,
GA4EventName> }) so that known GA4EventName values are forced to match the
GA4EventParamsMap lookup branch (refer to DataLayerEvent, GA4EventName,
GA4EventParamsMap).
| ( | ||
| command: "event", | ||
| name: string, | ||
| parameters?: Record<string, unknown>, | ||
| ): void; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the custom overload is currently unconstrained.
rg -n -C2 'command:\s*"event"|name:\s*string|Exclude<string,\s*GA4EventName>' core/types-google-analytics/src/index.tsRepository: stephansama/packages
Length of output: 398
🏁 Script executed:
# First, let's look at the complete Gtag type definition and understand the context
cat -n core/types-google-analytics/src/index.ts | head -200Repository: stephansama/packages
Length of output: 7631
🏁 Script executed:
# Find where GA4EventName is defined
rg -n 'GA4EventName' core/types-google-analytics/src/index.ts | head -20Repository: stephansama/packages
Length of output: 240
🏁 Script executed:
# Check imports and type definitions at the top of the file
head -50 core/types-google-analytics/src/index.tsRepository: stephansama/packages
Length of output: 2067
🏁 Script executed:
# Check if there are any tests or usage examples that might show the impact
rg -n 'gtag' core/types-google-analytics/src/index.tsRepository: stephansama/packages
Length of output: 206
🏁 Script executed:
# Look for any test files or examples to understand usage patterns
fd -e test -e spec -e example core/types-google-analyticsRepository: stephansama/packages
Length of output: 478
🏁 Script executed:
# Search for test files in the core/types-google-analytics directory
fd -e test -e spec -e example . core/types-google-analyticsRepository: stephansama/packages
Length of output: 46
🏁 Script executed:
# Check the package structure and see if there are any test files
ls -la core/types-google-analytics/Repository: stephansama/packages
Length of output: 489
Strengthen custom-event overload to exclude known GA4 event names from loose typing.
The second Gtag overload accepts name: string which includes known event names from GA4EventName. This allows calls like gtag('event', 'purchase', { invalid: true }) to match the unconstrained overload instead of the strict typed overload that enforces GA4EventParamsMap['purchase'].
Use Exclude<string, GA4EventName> on the second overload to force all known event names to the strict first overload:
Proposed fix
/** GA4 custom event with arbitrary params */
(
command: "event",
- name: string,
+ name: Exclude<string, GA4EventName>,
parameters?: Record<string, unknown>,
): void;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ( | |
| command: "event", | |
| name: string, | |
| parameters?: Record<string, unknown>, | |
| ): void; | |
| ( | |
| command: "event", | |
| name: Exclude<string, GA4EventName>, | |
| parameters?: Record<string, unknown>, | |
| ): void; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/types-google-analytics/src/index.ts` around lines 167 - 171, The loose
custom-event overload for the Gtag type currently allows any string (including
known GA4EventName) and should be narrowed so known event names are forced to
use the strongly-typed overload; update the second overload signature for Gtag
(the one with command: "event") to use name: Exclude<string, GA4EventName> so
that known GA4EventName values bind to the strict first overload that uses
GA4EventParamsMap, preventing accidental bypass of the typed params checks.
1 participant
Closes STE-56
New
@stephansama/google-analytics-typespackage. Types-only, covers both surfaces (GTMdataLayer.push+ GA4gtag('event', ...)) sharing one event taxonomy.Exports
Gtag— function signature with typed overloads on event name.DataLayerEvent— discriminated union oneventcovering all 18 recommended events; custom events fall through to{ event: string } & Record<string, unknown>.GA4EventParamsMap+ per-event interfaces (extend via module augmentation for custom events).GA4Item— shared ecommerce item shape.Window augmentation
Augments
Windowwith typeddataLayer+gtagproperties — no extra imports at call sites.Tests
4 type-level assertions via
expectTypeOfcovering gtag overload shape, custom-event acceptance, DataLayerEvent union, and the EventName↔Map identity.Acceptance criteria