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.

Sign up for GitHub

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

fix(script-element): Script is invalid in Head of NextJS #383

Open
GervinFung opened this issue Feb 8, 2023 · 7 comments
Open

fix(script-element): Script is invalid in Head of NextJS #383

GervinFung opened this issue Feb 8, 2023 · 7 comments
Assignees
@GervinFung

Comments

@GervinFung
Copy link

Hey, nice package u wrote, thanks for the effort. I wanna bring up a topic, not sure if anyone experienced it before, so here it goes, Script element of nextjs/script doesn't work when it's descendant of Head of next/head, I ended up using native script element instead. The reason I brought up this is because, it's recommended to put the google analytics script in head tag, see here. I think the fix is quite easy
@MauricioRobayo
Copy link
Owner

Hello @GervinFung !

Thanks for bringing this up.

Have you experienced any issues with the current implementation?

Would it be possible for you to verify if it is working properly as it is?

To verify that the tag is working, visit your website, and then check the Real-Time reports in Analytics to verify that your visit was registered.

You can also use Google Tag Assistant to determine whether your tag is implemented correctly.

Download Tag Assistant | Learn more about using Tag Assistant

Funny that for dynamic pages it says to add it after the opening <body> tag 🤷 :

Find the Google tag for your property and copy the code exactly without editing it.
Paste your Google tag into a file named "analyticstracking.php".
Include the analyticstracking.php file on each PHP template page.
For each template page, immediately after the opening <body> tag, add the following code:
<?php include_once("analyticstracking.php") ?>

Using the Scrip tag provides more control and, eventually, even using web-workers to load the script, which is an active feature request.

It seems that it is recommended in that way so it increases the likelihood that the tracking beacon will be sent before the user leaves the page, in cases where a user leaves the page before it is fully loaded.

We could have the user decide if they want to place it in a regular script tag to be used at the top of <head> and prioritize tracking users that leave on partial page loads, or using the Nextjs Script tag and prioritize performance.

@GervinFung
Copy link
Author

Thanks for replying! I appreciate it

To answer to questions u asked above, yes I've experienced issue and had verify it can't verify that I have visited my website

I've change it to native script element and it worked

I think we can build 1 more component to let user decide, I think I can do that if u don't mind

@MauricioRobayo
Copy link
Owner

I think we can build 1 more component to let user decide, I think I can do that if u don't mind

That would be awesome! Thanks @GervinFung , let me know if you need anything I'll be glad to help.

@MauricioRobayo
Copy link
Owner

@GervinFung do you think this could also be achieved using the beforeInteractive strategy, without the need for a new component:

https://nextjs.org/docs/api-reference/next/script#beforeinteractive

🤔

@timpetri timpetri mentioned this issue Feb 27, 2023
Closed
@GervinFung
Copy link
Author

@GervinFung do you think this could also be achieved using the beforeInteractive strategy, without the need for a new component:

https://nextjs.org/docs/api-reference/next/script#beforeinteractive

thinking

Sorry for the late reply, it won't work, even if I try to implement it with native element directly. It worked when I do a simple version with native a element like the image shown below
image

@p-prins
Copy link

p-prins commented Mar 23, 2023

FYI: Head.js will be deprecated: https://beta.nextjs.org/docs/api-reference/file-conventions/head#migration-guide .

Repository owner deleted a comment from wiah Mar 23, 2023
Repository owner deleted a comment from wiah Mar 23, 2023
@GervinFung
Copy link
Author

In that case, I think this means that this issue & PR is obsolete already, since @MauricioRobayo can write it with native head tag. Unless my PR replace NextJS Head with native head, that I am ok with it. Is this ok for you @MauricioRobayo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@GervinFung GervinFung

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@MauricioRobayo @GervinFung @p-prins