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

Jump to bottom

Move Google Analytics script to the <Head/> #43838

Merged
merged 2 commits into from Dec 13, 2022
Merged

Move Google Analytics script to the <Head/> #43838

merged 2 commits into from Dec 13, 2022

Conversation

aarnadlr
Copy link
Contributor

@aarnadlr aarnadlr commented Dec 8, 2022 •
edited

Fixes #37957

Google cannot read the Google Analytics script if it is placed at the bottom of the <body />.

It can only be read when it's placed here in the <Head />

This has been documented by other NextJS users online, and by Google themselves, which directs users to place the script in the head.

Hope this helps NextJS users!

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have a helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • e2e tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have a helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running pnpm build && pnpm lint
  • The "examples guidelines" are followed from our contributing doc

@aarnadlr
Move Google Analytics script to the <Head/>
66c8edf 
Google cannot read the Google Analytics script if it is placed at the bottom of the <body />.

It can only be read when it's placed here in the <Head />

This has been documented by other NextJS users online, and by Google themselves, which directs users to place the script in the head.

Hope this helps NextJS users!
@ijjk ijjk added the examples Issue/PR related to examples label Dec 8, 2022
@maxproske maxproske mentioned this pull request Dec 9, 2022
Merged
2 tasks
SukkaW
SukkaW approved these changes Dec 10, 2022
View reviewed changes
Copy link
Contributor

@SukkaW SukkaW left a comment •
edited

Choose a reason for hiding this comment

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

I believe the change is valid per Google Analytics support article.

Since the inline script tag is very lightweight (it only defines an array (window.dataLayer) and a function gtag()), this shouldn't impact Web Vitals. The main Google Tag Manager script is still loaded via next/script component.

@Josehower
Copy link
Contributor

Josehower commented Dec 12, 2022 •
edited

But in this case, the documentation is suggested to load both scripts on HEAD.

image

image

After some tests, Google recognizes the tag even when both scripts are at the end of the body. @SukkaW Do you think this usage of script inside of HEAD is absolutely necessary?

@kodiakhq kodiakhq bot merged commit ee266b9 into vercel:canary Dec 13, 2022
timneutkens pushed a commit that referenced this pull request Dec 13, 2022
@aarnadlr
Move Google Analytics script to the <Head/> (#43838)
3e7cca0 
Fixes #[37957](#37957)

Google cannot read the Google Analytics script if it is placed at the bottom of the `<body />`.

It can only be read when it's placed here in the `<Head />`

This has been documented by other NextJS users online, and by Google themselves, which directs users to place the script in the head.

Hope this helps NextJS users!



## Bug

- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
@dohomi
Copy link

dohomi commented Dec 16, 2022

Hey guys this change duplicates the code into Head and before end of Body. The code is now present in both _app and _document.

@Josehower
Copy link
Contributor

Hey guys this change duplicates the code into Head and before end of Body. The code is now present in both _app and _document.

There is a open PR that solves this #43894

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ijjk ijjk ijjk approved these changes

@SukkaW SukkaW SukkaW approved these changes

@timneutkens timneutkens Awaiting requested review from timneutkens

@shuding shuding Awaiting requested review from shuding

@leerob leerob Awaiting requested review from leerob

@steven-tey steven-tey Awaiting requested review from steven-tey

@balazsorban44 balazsorban44 Awaiting requested review from balazsorban44

Assignees
No one assigned
Labels
examples Issue/PR related to examples
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@aarnadlr @Josehower @dohomi @ijjk @SukkaW