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

next/head order changed #34885

Closed
1 task done
bobaaaaa opened this issue Feb 28, 2022 · 12 comments
Closed
1 task done

next/head order changed #34885

bobaaaaa opened this issue Feb 28, 2022 · 12 comments
Labels
bug Issue was opened via the bug report template. Metadata Related to Next.js' Metadata API. Script (next/script) Related to Next.js Script Optimization.

Comments

@bobaaaaa
Copy link
Contributor

Verify canary release

  • I verified that the issue exists in Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 21.2.0: Sun Nov 28 20:28:54 PST 2021; root:xnu-8019.61.5~1/RELEASE_X86_64
Binaries:
  Node: 16.13.0
  npm: 8.1.0
  Yarn: 1.22.17
  pnpm: N/A
Relevant packages:
  next: 12.1.0
  react: 17.0.2
  react-dom: 17.0.2

What browser are you using? (if relevant)

firefox, chrome

How are you deploying your application? (if relevant)

No response

Describe the Bug

If I add a <Head /> component in _app.js, _document.js and a page, the order of tags changed from next 12.0.10 to 12.1.0. I created an example repo.

head order next@canary

v12.1.1-canary.4

<meta content="_app.js" />
<meta content="index.js" />
<meta content="_document.js" />

head order next 12.1.0

<meta content="_app.js" />
<meta content="index.js" />
<meta content="_document.js" />

head order next 12.0.10

<meta content="_document.js" />
<meta content="_app.js" />
<meta content="index.js" />

Expected Behavior

It's either a bug or this "feature" is not mentioned anywhere (release notes). In my opinion, the next 12.0.10 version is correct and this is a bug.

To Reproduce

https://github.com/bobaaaaa/next-bug-head
@bobaaaaa bobaaaaa added the bug Issue was opened via the bug report template. label Feb 28, 2022
@balazsorban44
Copy link
Member

Are you seeing any issues that are a consequence of this?

@bobaaaaa
Copy link
Contributor Author

yes, we had some script order issues. We moved the specific code to _app.tsx. This worked, but now we have an increased bundlesize.

@balazsorban44
Copy link
Member

balazsorban44 commented Feb 28, 2022 •
edited

I believe this is the relevant PR: #28119

Could you share your code?

@balazsorban44 balazsorban44 added the Metadata Related to Next.js' Metadata API. label Feb 28, 2022
@bobaaaaa
Copy link
Contributor Author

Here is a simplified version for our consent management (broken version):

<!-- _document.tsx -->
<script>
  window.loadOnConsent = (listOfScripts) => {}; // a script loader
</script>
// _app.tsx
<Head>
  <script dangerouslySetInnerHTML={{ __html: `window.loadOnConsent(${JSON.stringify(getScriptsThatNeedConsent(config))})` }} />
</Head>

I know there is the next/script component. For us, it is important that we load the consent script as fast as possible. That's why we optimize the loading + execution timings as much as possible. We do this "outside" of react/next and communicate with a global api.

@balazsorban44 balazsorban44 added the Script (next/script) Related to Next.js Script Optimization. label Feb 28, 2022
@balazsorban44
Copy link
Member

For this exact case there is the beforeInteractive strategy of next/script. Why wouldn't that work? 🤔

Scripts that load with the beforeInteractive strategy are injected into the initial HTML from the server and run before self-bundled JavaScript is executed. This strategy should be used for any critical scripts that need to be fetched and executed before the page is interactive.

https://nextjs.org/docs/basic-features/script#beforeinteractive

@bobaaaaa
Copy link
Contributor Author

bobaaaaa commented Feb 28, 2022 •
edited

@balazsorban44 it has nothing to do with next/script :). I only mentioned it because it is the nextjs-standard answer for inlining javascript. But next/script did not work for us.

@balazsorban44
Copy link
Member

It is, because it gives the guarantees for your requirements. I don't understand why it would not work, as the mentioned strategy is optimized for your use case.

But to make sure if we have a different recommendation, I'll ask the rest of the team. 👍

@bobaaaaa
Copy link
Contributor Author

Here are some reasons why next/script did not work for us:

  1. next/script did not work in _document.tsx
  2. inlining code with next/script == writing code in strings. This works okay for some configs, but not for a lot of functions.
  3. you can not easily test code in strings
  4. inlined code will also land in the main bundle (waste of bytes)

I have a wishlist for next/script :)

  1. works in _document.tsx
  2. <Script webpackEntryPoint="./path-to-my-script.ts" />

@KingMatrix1989
Copy link

Google Tag Manager snippet code was inserted in my case at the end of other meta tags, which caused the same problem.
The problem was solved after moving GTM snippet code to the top of meta tags.

@LukasBombach
Copy link

I have a wishlist for next/script :)

1. works in `_document.tsx`

2. `<Script webpackEntryPoint="./path-to-my-script.ts" />`

Hi, colleague of @bobaaaaa 's here, this is what we came up with:

https://github.com/LukasBombach/next-inline-script

we are using a child compiler and a webpack loader to compile and place some parts of our code in the <head>

@jankaifer
Copy link
Contributor

This is a bug report of unexpected change, but it's not actually a bug in itself.
Since that change happened quite a long time ago, it should be safe to close this now.

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Issue was opened via the bug report template. Metadata Related to Next.js' Metadata API. Script (next/script) Related to Next.js Script Optimization.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@bobaaaaa @LukasBombach @KingMatrix1989 @jankaifer @balazsorban44