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

[NEXT-1304] <Script> with beforeInteractive strategy ignores additional attributes in app router #49830

Closed
1 task done
victorandree opened this issue May 15, 2023 · 17 comments · Fixed by #59779
Closed
1 task done
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked

Comments

@victorandree
Copy link

victorandree commented May 15, 2023

Note: Updated on August 24, 2023, with latest version of Next.js.

Verify canary release

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

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 22.6.0: Wed Jul  5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000
Binaries:
  Node: 20.5.1
  npm: 9.8.0
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 13.4.20-canary.4
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.6
Next.js Config:
  output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

App directory (appDir: true), Script optimization (next/script)

Link to the code that reproduces this issue

https://github.com/victorandree/next-script-before-interactive-attributes-app-dir

To Reproduce

Setup a project with the app directory and in the root layout, use <Script> with strategy="beforeInteractive" and pass additional attributes. The additional attributes are not included in the resulting script tag.

Describe the Bug

When using the <Script> tag from next/script in the root layout of the app directory, additional attributes are not included if you use strategy="beforeInteractive":

import Script from 'next/script';

export default function RootLayout({ children }) {
  return (
    <html>
      <head />
      <body>
        {children}
        <Script
          strategy="beforeInteractive"
          src="https://example.com/script.js"
          id="example-script"
          nonce="XUENAJFW"
          data-test="script"
        />
      </body>
    </html>
  );
}

A second point is that the <script> tag is not part of the HTML from the server, but is only added as (1) a <link rel="preload"> tag and (2) to a self.__next_s array and (3) as part of server component 'chunks', before it's rendered into the DOM.

Including such a script tag in pages/_document will render it with additional attributes (not reproduced) and as a script tag "directly". This issue therefore only affects the app directory.

A possible workaround is to include the script tags directly in head of your root layout:

export default function RootLayout({ children }) {
  return (
    <html>
      <head>
        <script
          src="https://example.com/script.js"
          id="example-script"
          nonce="XUENAJFW"
          data-test="script"
        />
      </head>
      <body>{children}</body>
    </html>
  );
}

For visibility, I encountered this while trying to install the OneTrust cookie consent snippet on a Next.js site, which requires passing an identifier as data-domain-script to a script tag.

Expected Behavior

The documented behavior of the <Script> tag is that it includes additional attributes (see "Additional Attributes" on the "Script Optimization" page of the App router documentation).

I also think it's a bit strange that the script tag is not rendered directly to the HTML from the server, since "before interactive" scripts are typically used for consent management (that's the example use case provided by the documentation), where it's important that they're inserted early.

I'd expect additional attributes to be included, even if I use it in the app directory.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

NEXT-1304

@victorandree victorandree added the bug Issue was opened via the bug report template. label May 15, 2023
@github-actions github-actions bot added the area: app App directory (appDir: true) label May 15, 2023
@maxstoyanov
Copy link

I just ran into this issue with 13.4.4.

I also think it's a bit strange that the script tag is not rendered directly to the HTML from the server, since "before interactive" scripts are typically used for consent management (that's the example use case provided by the documentation), where it's important that they're inserted early.

I was quite surprised about this. The beforeInteractive strategy should include the script early in the head tag. It must be written without running scripts in the main thread.

@tappress
Copy link

Next 13.4.4

Debugging half a day, I have several points to add. All with App Directory:

  1. Script with beforeInteractive is successfully added inside <head> as described in docs, but only in DEV mode. In PROD (next start) it just sits in form of self.__next_s=self.__next_s||[]).push... as mentioned by @victorandree.

A possible workaround is to include the script tags directly in head of your root layout

  1. This also works in development, but not in production. Even though <script> is being added in DOM by next in the manner smilar to beforeInteractive (e.g. self.__next_s=self.__next_s||[]).push...) it is not executed.

@victorandree
Copy link
Author

victorandree commented Jun 11, 2023

@prepwork Hm, I just tried this out on 13.4.5, including the workaround (see victorandree/next-script-before-interactive-attributes-app-dir@3140d39).

  1. The bug described in the issue is still present, i.e. additional attributes are not passed to the (eventual) script tag. Unlike your first point, I am seeing a script tag in head once Next.js has processed self.__next_s.
  2. To me, the workaround seems to work even in production (i.e. npm run build && npm run start).

For reference, this is what the start of my server-rendered HTML looks like (when running HEAD of my reproduction repository using npm run start; I've formatted the HTML for readability). https://example2.com/script2.js is inserted with a workaround, https://example.com/script.js with Script.

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8" />
    <link rel="preload" as="script" href="https://example2.com/script2.js" />
    <link rel="preload" as="script" href="https://example.com/script.js" />
    <meta name="viewport" content="width=device-width, initial-scale=1" />
    <script src="https://example2.com/script2.js" data-test="script2"></script>
    <script
      src="/_next/static/chunks/polyfills-78c92fac7aa8fdd8.js"
      nomodule=""
    ></script>
  </head>
  <body>
    <script>
      (self.__next_s = self.__next_s || []).push([
        'https://example.com/script.js',
      ]);
    </script>

@tappress
Copy link

tappress commented Jun 12, 2023

@victorandree, I've tried your project, and indeed, scripts are executing after being processed by self.__next_s, BUT not in all cases. I shredded my code to pieces and found out that it's enough to use useSearchParams() somewhere inside your page or component and beforeInteractive scripts or script added directly via <script> tag won't execute in production (next start):

"use client"
import { useSearchParams } from "next/navigation"

export default function Home() {
  const searchParams = useSearchParams()
  return <div>HOME PAGE</div>
}

Just comment out line with useSearchParams() and it starts working again:

"use client"
import { useSearchParams } from "next/navigation"

export default function Home() {
  // const searchParams = useSearchParams()
  return <div>HOME PAGE</div>
}

Have no clue why it is so.

@victorandree
Copy link
Author

@prepwork I tried this out, and if I add useSearchParams, I get this warning:

Entire page / deopted into client-side rendering. https://nextjs.org/docs/messages/deopted-into-client-rendering /

I can verify that when this happens, the self.__next_s script is never "converted" into a script tag. The workaround is converted (with additional attributes), but is not part of the HTML sent from the server.

IMO, this is technically another bug - fundamentally the same issue as #48335 (which is about static rendering of meta tags). The workaround there is to wrap useSearchParams in Suspense (which I don't think is obvious).

@tappress
Copy link

@victorandree, thanks a lot! It helped me greatly!

@leerob leerob added the linear: next Confirmed issue that is tracked by the Next.js team. label Jun 22, 2023
@timneutkens timneutkens changed the title <Script> with beforeInteractive strategy ignores additional attributes in app router [NEXT-1304] <Script> with beforeInteractive strategy ignores additional attributes in app router Jun 22, 2023
@DanielSpindler
Copy link

@prepwork i dont know if this has to do with a recent change, but you should also consider this from nextJS https://nextjs.org/docs/messages/no-before-interactive-script-outside-document Seems to be not really a bug, its just not intended that way

@desnor
Copy link

desnor commented Aug 24, 2023

@prepwork i dont know if this has to do with a recent change, but you should also consider this from nextJS https://nextjs.org/docs/messages/no-before-interactive-script-outside-document Seems to be not really a bug, its just not intended that way

with regard to this, is this still relevant when using the app router? it seems to me like something that just hasn't been updated yet as it seems conflicting to try and include a pages/_document.tsx to define the base when also using a layout.tsx root layout file. Does anyone know what the story is with this?

@victorandree
Copy link
Author

@desnor @prepwork I agree that this isn't relevant for the app router. I don't see that warning when using the app router. The app router docs for <Script> even contain an example of using beforeInteractive in the root layout, so I definitely think this is supported: https://nextjs.org/docs/app/api-reference/components/script#beforeinteractive.

FWIW, I've updated my reproduction repository to next@13.4.20-canary.4 and the issue is still present.

@azar-xp
Copy link

azar-xp commented Sep 16, 2023

This seems to be a bug.

Official docs for /app https://nextjs.org/docs/app/api-reference/components/script#beforeinteractive says that it's before and after interactive is supported from next 13.0.0 but not for build script.

Proposed fix on docs for /app https://nextjs.org/docs/messages/no-before-interactive-script-outside-document is not relevant because it's conflicting with /app/layout.tsx and not working.

The only workaround that I was able to make work to use standard html script tag and use defer on loading js script:

<html lang="en">
<body>
    <script
          defer
          type="text/javascript"
          src="https://www.termsfeed.com/public/cookie-consent/4.1.0/cookie-consent.js"
        ></script>
        <script
          type="text/javascript"
          dangerouslySetInnerHTML={{
            __html: `document.addEventListener('DOMContentLoaded', function () {
cookieconsent.run({"notice_banner_type":"interstitial","consent_type":"express","palette":"light","language":"en","page_load_consent_levels":["strictly-necessary"],"notice_banner_reject_button_hide":false,"preferences_center_close_button_hide":false,"page_refresh_confirmation_buttons":false,"website_name":"Name"});
          });`,
          }}
        ></script>
    {children}
    </body>
</html>

@djacquemard
Copy link

Hello !

Same issue here trying to add OneTrust script with beforeInteractive strategy.
Thank you for the workaround @victorandree !

Did you add the OneTrust Cookie List to your website ? On my side, trying to add <div id="ot-sdk-cookie-policy" /> container cause (of course) hydration error and I can't solve this by any way.

@shaunforage
Copy link

Experiencing the same issue on 13.5.6.

@jesse-mm
Copy link

Confirmed still not working as advertised in 14.0.2. I've put the script I want to load in the RootLayout as described in the docs. But it still loads next modules first, after that it injects the custom script.

Following the documentation this should not happen:

beforeInteractive: Load before any Next.js code and before any page hydration occurs.

@fmontes
Copy link
Contributor

fmontes commented Dec 5, 2023

Can confirm this is happening in 14 for me as well.

@zbauman3
Copy link
Contributor

I'm on 14.0.4 and can confirm this is still an issue.

@zbauman3
Copy link
Contributor

I've created a PR to fix this issue. I've tested locally and can confirm this solves the problem. Now just waiting on Next.js folks to review.

#59779

huozhi pushed a commit that referenced this issue Jan 3, 2024
…attributes in App Router (#59779)

### What?

Currently, `next/script` in the App Router does not behave as the docs
describe in the [Additional
Attributes](https://nextjs.org/docs/app/building-your-application/optimizing/scripts#additional-attributes)
section – it does not pass on all additional attributes when
`strategy="beforeInteractive"`
([#49830](#49830)).

### Why?

The props from the `<Script>` component were not passed on to the
underlying script loader (`self.__next_s`).

### How?

I've added the missing props to the object that is `push`ed to
`self.__next_s`.

NEXT-1304
Fixes #49830
agustints pushed a commit to agustints/next.js that referenced this issue Jan 6, 2024
…attributes in App Router (vercel#59779)

### What?

Currently, `next/script` in the App Router does not behave as the docs
describe in the [Additional
Attributes](https://nextjs.org/docs/app/building-your-application/optimizing/scripts#additional-attributes)
section – it does not pass on all additional attributes when
`strategy="beforeInteractive"`
([vercel#49830](vercel#49830)).

### Why?

The props from the `<Script>` component were not passed on to the
underlying script loader (`self.__next_s`).

### How?

I've added the missing props to the object that is `push`ed to
`self.__next_s`.

NEXT-1304
Fixes vercel#49830
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. 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 Jan 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked
Projects
None yet