fix(gtm): don't declare dataLayer on the global Window (1.x backport) - #865
Conversation
Co-authored-by: Harlan Wilton <harlan@harlanzw.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan includes up to 4 reviews per rolling hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe global Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized type-only fix removes the conflicting global 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
commit: |
🔗 Linked issue
Backport of #855 to the
1.xrelease line. Fixes #852 on1.x.❓ Type of change
📚 Description
Straight cherry-pick of a6d763e, no conflicts and no drift.
1.xstill exports the deprecatedGoogleTagManagerConsentalias thatmaindropped, but the cherry-pick does not touch that line.The GTM registry augmented the global
Windowwith the wholeGoogleTagManagerApi, including a requireddataLayer. That made the package irreconcilable with any other package that declaresWindow.dataLayer, most commonly@gtm-support/core, the engine behind@gtm-support/vue-gtm. The two declarations merge into oneWindow, which then fails this package's ownextendsclause. TypeScript reports TS2430 at every one of the consumer's ownWindowaugmentations, and the consumer cannot suppress it because neither declaration is theirs.Making
dataLayeroptional does not fix it.Array<T>.pushis(...items: T[]) => number, whileDataLayerPush's first overload starts with(command: string, ...), so a foreign element type stays incompatible.The fix declares only the member that is genuinely global and unambiguous:
GoogleTagManagerApiitself is unchanged, souseScriptGoogleTagManager()and its proxy keepdataLayertyped. The registry never read the global declaration fordataLayeranyway: it reads(window as any)[dataLayerName], because the name is configurable through thel/dataLayeroptions.window.google_tag_manageris read directly inuse(), which is why that member stays.This is a type-level change only. Zero runtime bytes.
📝 Checklist
test/types/global-window.test-d.tsguards the regression. On this branch it fails against the old declaration and passes against the fix.On
1.x:pnpm vitest run --project typecheckpasses (46 tests, no type errors),pnpm typecheckclean,pnpm lintclean.