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/script not working in custom Document when outside of next/document's <Head> #37741

Closed
1 task done
aboqasem opened this issue Jun 16, 2022 · 5 comments · Fixed by #37894
Closed
1 task done

next/script not working in custom Document when outside of next/document's <Head> #37741

aboqasem opened this issue Jun 16, 2022 · 5 comments · Fixed by #37894
Labels
Script (next/script) Related to Next.js Script Optimization.

Comments

@aboqasem
Copy link

aboqasem commented Jun 16, 2022 •
edited
Loading

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.5.0: Tue Apr 26 21:08:22 PDT 2022; root:xnu-8020.121.3~4/RELEASE_X86_64
Binaries:
  Node: 16.15.0
  npm: 8.12.2
  Yarn: 1.22.18
  pnpm: 7.2.1
Relevant packages:
  next: 12.1.7-canary.40
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

next start

Describe the Bug

I am trying to get <Script strategy="beforeInteractive" /> to work, I did exactly what the docs say, but the script is not present in the built index.html nor working in next dev.

Expected Behavior

<Script strategy="beforeInteractive" /> to work.

To Reproduce

  1. Initialize a next project
  2. Add the example code from the docs in _document.js
  3. Run and check DevTools > Network
  4. No request will be found to https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js

Or from a shell terminal:

cd /tmp && npx create-next-app tmp && cd tmp
tee pages/_document.js <<'EOF' >/dev/null
import { Html, Head, Main, NextScript } from 'next/document'
import Script from 'next/script'

export default function Document() {
  return (
    <Html>
      <Head />
      <body>
        <Main />
        <NextScript />
        <Script
          src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js"
          strategy="beforeInteractive"
        ></Script>
      </body>
    </Html>
  )
}
EOF
yarn add next@canary && yarn dev
@aboqasem aboqasem added the bug Issue was opened via the bug report template. label Jun 16, 2022
@aboqasem aboqasem mentioned this issue Jun 16, 2022
Closed
@aboqasem
Copy link
Author

aboqasem commented Jun 16, 2022 •
edited
Loading

It seems that there are succeeding tests for this:

https://github.com/vercel/next.js/blob/ea89854e3297364fcd44e3ae77adffe36b2be54e/test/integration/script-loader/test/index.test.js

https://github.com/vercel/next.js/blob/ea89854e3297364fcd44e3ae77adffe36b2be54e/test/integration/script-loader/base/pages/_document.js

Related:

#31936

#36364

#37000

@balazsorban44 balazsorban44 added the Script (next/script) Related to Next.js Script Optimization. label Jun 16, 2022
@housseindjirdeh
Copy link
Collaborator

There's an issue currently that requires beforeInteractive scripts to live inside <Head> in _document.js:

import { Html, Head, Main, NextScript } from 'next/document'
import Script from 'next/script'

export default function Document() {
  return (
    <Html>
      <Head>
       <Script
         src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js"
         strategy="beforeInteractive"
       ></Script>
      </Head>
      <body>
        <Main />
        <NextScript />
      </body>
    </Html>
  )
}

I'll submit a PR next week that removes this restriction and allow you to place <Script> anywhere in _document.js, but you can move it to <Head> for the meantime to make it work :)

@aboqasem
Copy link
Author

Thank you, @housseindjirdeh! Yeah, I can verify that.

Is it ok if I try to work on this and submit a PR?

@aboqasem aboqasem changed the title <Script strategy="beforeInteractive" /> not doing anything in _document.js <Script strategy="beforeInteractive" /> not doing anything in _document.js when outside of <Head> Jun 16, 2022
@housseindjirdeh
Copy link
Collaborator

@aboqasem Go for it! I was meaning to work on it next week but will be glad to help you and review your PR instead :)

@balazsorban44 balazsorban44 added kind: bug and removed bug Issue was opened via the bug report template. labels Jun 16, 2022
aboqasem added a commit to aboqasem/next.js that referenced this issue Jun 16, 2022
@aboqasem
test(next/script): failing beforeInteractive tests for bug vercel#3…
5cdc750 
…7741
@aboqasem aboqasem mentioned this issue Jun 16, 2022
Closed
3 tasks
@aboqasem aboqasem changed the title <Script strategy="beforeInteractive" /> not doing anything in _document.js when outside of <Head> next/script not working in custom Document when outside of next/document's <Head> Jun 17, 2022
aboqasem added a commit to aboqasem/next.js that referenced this issue Jun 18, 2022
@aboqasem
revert: "test(next/script): failing beforeInteractive tests for bug v…
7a8b4c5 
…ercel#37741"

This reverts commit 5cdc750.
aboqasem added a commit to aboqasem/next.js that referenced this issue Jun 18, 2022
@aboqasem
test(next/script): failing beforeInteractive tests for bug vercel#3…
f6997b1 
…7741
aboqasem added a commit to aboqasem/next.js that referenced this issue Jun 18, 2022
@aboqasem
revert: "test(next/script): failing beforeInteractive tests for bug v…
1e22020 
…ercel#37741"

This reverts commit 5cdc750.
aboqasem added a commit to aboqasem/next.js that referenced this issue Jun 18, 2022
@aboqasem
test(next/script): failing tests for bug vercel#37741
990b58a
@housseindjirdeh housseindjirdeh mentioned this issue Jun 21, 2022
Merged
3 tasks
kodiakhq bot added a commit to housseindjirdeh/next.js that referenced this issue Jun 27, 2022
@kodiakhq
Merge branch 'canary' into bug/vercel#37741
d5f7b73
@kodiakhq kodiakhq bot closed this as completed in #37894 Jun 27, 2022
kodiakhq bot pushed a commit that referenced this issue Jun 27, 2022
@housseindjirdeh
[Script] Allow next/script to be placed in _document body (#37894)
82dce6c 
Fixes #37741. Allows `next/script` to be placed inside of `_document` body in addition to `Head`.

## Bug

- [X] Related issues linked using `fixes #number`
- [X] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
@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 Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Script (next/script) Related to Next.js Script Optimization.
Projects
None yet
Milestone
No milestone
3 participants
@housseindjirdeh @balazsorban44 @aboqasem