Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding <GoogleTagManager> component to @next/third-parties #56106

Merged
merged 31 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
733a173
Adding useGoogltTagManager hook to @next/third-parties
janicklas-ralph Sep 27, 2023
022a773
Adding sendData function
janicklas-ralph Sep 27, 2023
21551de
Adding tests
janicklas-ralph Sep 27, 2023
d6d7944
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Sep 27, 2023
2a74723
Fix lint
janicklas-ralph Sep 27, 2023
52ca4e2
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Sep 27, 2023
ee5969f
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Sep 27, 2023
63ddaf7
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Sep 27, 2023
fee976a
Fix lint
janicklas-ralph Sep 27, 2023
07a8f15
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Sep 28, 2023
ff6b24f
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 9, 2023
e8dd1cc
Update GTM component
janicklas-ralph Oct 10, 2023
33db325
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 10, 2023
8f143c8
Update app-dir tests
janicklas-ralph Oct 10, 2023
ed30df7
Update sendEvent function
janicklas-ralph Oct 10, 2023
7f9cf1c
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 10, 2023
e5473fc
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 11, 2023
5785971
Update sendEvent function
janicklas-ralph Oct 11, 2023
9c2fd1f
Fix import
janicklas-ralph Oct 11, 2023
49e29a6
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 11, 2023
3c9fc3f
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 12, 2023
f84dcae
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 13, 2023
ae0463b
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 16, 2023
4b6b00c
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 16, 2023
86d1a29
Update filenames
janicklas-ralph Oct 16, 2023
1e3031d
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 16, 2023
0fe8864
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 16, 2023
0a3c63b
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 16, 2023
fbd24de
Merge branch 'canary' of github.com:vercel/next.js into gtm
janicklas-ralph Oct 17, 2023
fa14ece
Fix updating dataLayerName
janicklas-ralph Oct 17, 2023
0ecadb9
Fix updating dataLayerName and types
janicklas-ralph Oct 17, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/third-parties/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"name": "@next/third-parties",
"version": "13.5.5",
"private": true,
"repository": {
"url": "vercel/next.js",
"directory": "packages/third-parties"
Expand Down
59 changes: 59 additions & 0 deletions packages/third-parties/src/google/gtm.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
'use client'
// TODO: Evaluate import 'client only'
import React from 'react'
import Script from 'next/script'

declare global {
interface Window {
dataLayer?: Object[]
[key: string]: any
}
}

type GTMParams = {
gtmId: string
dataLayer: string[]
dataLayerName: string
auth: string
preview: string
}

let currDataLayerName = 'dataLayer'

export function GoogleTagManager(props: GTMParams) {
const { gtmId, dataLayerName = 'dataLayer', auth, preview, dataLayer } = props
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we rename gtmId to just id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally had it as id but since I made it into a component, I am accepting the GTM params as props

<GoogleTagManager gtmId="GTM_XYZ" />

In this case id might clash with HTML id so I changed it to gtmID


currDataLayerName = dataLayerName
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this pattern is edge-case free. When you render this component it will use the initial dataLayer prop to instantiate the script by id. Then afterwards any changes to the dataLayer prop will not be reflected in teh gtm script but will alter this module scoped variable.

The sendGTMEvent refers to the current dataLayer rather than the the one initially used instantiate the script so a wayward props update could breat gtm tracking.

If it is true that you can't hot-update the datalayer in GTM (meaning you need to just pick it at the start of the app runtime and never change it) then we should align the module global to only update when the gtm script will actually run which is just once one hte first render.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense! I have updated the code to only modify currDataLayerName during initialization


const gtmLayer = dataLayerName !== 'dataLayer' ? `$l=${dataLayerName}` : ''
const gtmAuth = auth ? `&gtm_auth=${auth}` : ''
const gtmPreview = preview ? `&gtm_preview=${preview}&gtm_cookies_win=x` : ''

return (
<>
<Script
id="_next-gtm-init"
dangerouslySetInnerHTML={{
__html: `
(function(w,l){
w[l]=w[l]||[];
w[l].push({'gtm.start': new Date().getTime(),event:'gtm.js'});
${dataLayer ? `w[l].push(${JSON.stringify(dataLayer)})` : ''}
})(window,'${dataLayerName}');`,
}}
></Script>
huozhi marked this conversation as resolved.
Show resolved Hide resolved
<Script
id="_next-gtm"
src={`https://www.googletagmanager.com/gtm.js?id=${gtmId}${gtmLayer}${gtmAuth}${gtmPreview}`}
></Script>
</>
)
}

export const sendGTMEvent = (data: Object) => {
if (window[currDataLayerName]) {
window[currDataLayerName].push(data)
} else {
console.warn(`dataLayer ${currDataLayerName} does not exist`)
huozhi marked this conversation as resolved.
Show resolved Hide resolved
}
}
1 change: 1 addition & 0 deletions packages/third-parties/src/google/index.tsx
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export { default as GoogleMapsEmbed } from './GoogleMapsEmbed'
janicklas-ralph marked this conversation as resolved.
Show resolved Hide resolved
huozhi marked this conversation as resolved.
Show resolved Hide resolved
export { default as YouTubeEmbed } from './YouTubeEmbed'
export { GoogleTagManager, sendGTMEvent } from './gtm'
23 changes: 23 additions & 0 deletions test/e2e/app-dir/third-parties/app/gtm/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use client'

import React from 'react'
import { GoogleTagManager, sendGTMEvent } from '@next/third-parties/google'

const Page = () => {
const onClick = () => {
sendGTMEvent({ event: 'buttonClicked', value: 'xyz' })
}

return (
<div class="container">
<GoogleTagManager gtmId="GTM-XYZ" />
<h1>GTM</h1>
<button id="gtm-send" onClick={onClick}>
Click
</button>
<GoogleTagManager gtmId="GTM-XYZ" />
</div>
)
}

export default Page
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can do this in a separate PR, but we should update the GTM example in the examples/ repo to show this newer approach

Copy link

Choose a reason for hiding this comment

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

Is the double <GoogleTagManager gtmId="GTM-XYZ" /> on line 13 and 18 an oversight? Same in test/e2e/third-parties/pages/gtm.js

Copy link
Collaborator

Choose a reason for hiding this comment

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

I had thought the same in the beginning, but I recall @janicklas-ralph mentioning that it is was deliberate to test that the behavior if an identical GTM containers is included by accident

@janicklas-ralph Can you clarify, and if so, add a note here in a future PR? (not urgent)

25 changes: 25 additions & 0 deletions test/e2e/app-dir/third-parties/basic.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createNextDescribe } from 'e2e-utils'
import { waitFor } from 'next-test-utils'

createNextDescribe(
'@next/third-parties basic usage',
Expand Down Expand Up @@ -29,5 +30,29 @@ createNextDescribe(
expect(baseContainer.length).toBe(1)
expect(mapContainer.length).toBe(1)
})

it('renders GTM', async () => {
const browser = await next.browser('/gtm')

await browser.waitForElementByCss('#_next-gtm')
await waitFor(1000)

const gtmInlineScript = await browser.elementsByCss('#_next-gtm-init')
expect(gtmInlineScript.length).toBe(1)

const gtmScript = await browser.elementsByCss(
'[src^="https://www.googletagmanager.com/gtm.js?id=GTM-XYZ"]'
)

expect(gtmScript.length).toBe(1)

const dataLayer = await browser.eval('window.dataLayer')
expect(dataLayer.length).toBe(1)

await browser.elementByCss('#gtm-send').click()

const dataLayer2 = await browser.eval('window.dataLayer')
expect(dataLayer2.length).toBe(2)
})
}
)
25 changes: 25 additions & 0 deletions test/e2e/third-parties/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createNextDescribe } from 'e2e-utils'
import { waitFor } from 'next-test-utils'

createNextDescribe(
'@next/third-parties basic usage',
Expand Down Expand Up @@ -28,5 +29,29 @@ createNextDescribe(
expect(baseContainer.length).toBe(1)
expect(mapContainer.length).toBe(1)
})

it('renders GTM', async () => {
const browser = await next.browser('/gtm')

await browser.waitForElementByCss('#_next-gtm')
await waitFor(1000)

const gtmInlineScript = await browser.elementsByCss('#_next-gtm-init')
expect(gtmInlineScript.length).toBe(1)

const gtmScript = await browser.elementsByCss(
'[src^="https://www.googletagmanager.com/gtm.js?id=GTM-XYZ"]'
)

expect(gtmScript.length).toBe(1)

const dataLayer = await browser.eval('window.dataLayer')
expect(dataLayer.length).toBe(1)

await browser.elementByCss('#gtm-send').click()

const dataLayer2 = await browser.eval('window.dataLayer')
expect(dataLayer2.length).toBe(2)
})
}
)
21 changes: 21 additions & 0 deletions test/e2e/third-parties/pages/gtm.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import React from 'react'
import { GoogleTagManager, sendGTMEvent } from '@next/third-parties/google'

const Page = () => {
const onClick = () => {
sendGTMEvent({ event: 'buttonClicked', value: 'xyz' })
}

return (
<div class="container">
<GoogleTagManager gtmId="GTM-XYZ" />
<h1>GTM</h1>
<button id="gtm-send" onClick={onClick}>
Click
</button>
<GoogleTagManager gtmId="GTM-XYZ" />
</div>
)
}

export default Page
Loading