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

Script component results in different attributes based on the strategy #33686

Open
albertodeago opened this issue Jan 26, 2022 · 13 comments
Open
Assignees
Labels
bug Issue was opened via the bug report template. Script (next/script) Related to Next.js Script Optimization.

Comments

@albertodeago
Copy link

Run next info (available from version 12.0.8 and up)

next info output:

Operating System:
      Platform: darwin
      Arch: x64
      Version: Darwin Kernel Version 20.6.0: Wed Nov 10 22:23:07 PST 2021; root:xnu-7195.141.14~1/RELEASE_X86_64
    Binaries:
      Node: 16.13.0
      npm: 8.1.0
      Yarn: 1.22.11
      pnpm: 6.24.2
    Relevant packages:
      next: 12.0.8
      react: 17.0.2
      react-dom: 17.0.2

What version of Next.js are you using?

12.0.8

What version of Node.js are you using?

16.13.0

What browser are you using?

Chrome

What operating system are you using?

macOS

How are you deploying your application?

next start

Describe the Bug

I premise that I am not sure it is a problem, however I have not found this behavior documented anywhere.

I'm using the Script component and I'm seeing some differences when setting the strategy to beforeInteractive compared to other strategies.
The main difference I'm seeing is on the defer and crossorigin attributes. I set corssorigin=anonymous and defer=false in some of our scripts.
Setting beforeInteractive strategy the ending DOM element always has the defer attribute (no matter what I pass to the component) and the crossorigin is ignored.

Examples:
with code like this

<Script
  src="test.js"
  id="test"
  crossOrigin="anonymous"
  data-test="test"
  defer={false}
  strategy="beforeInteractive"
/>

the output is

<script src="test.js" id="test" data-test="test" defer="" data-nscript="beforeInteractive"></script>

with code like this

<Script
  src="test.js"
  id="test"
  crossOrigin="anonymous"
  data-test="test"
  defer={false}
  strategy="afterInteractive"
/>

the output is

<script src="test.js" id="test" crossorigin="anonymous" data-test="test" defer="false" data-nscript="afterInteractive"></script>

Expected Behavior

defer and crossorigin attributes are not changed based on strategy

To Reproduce

I created a repository to reproduce it easily.
Just clone it, npm run dev, and open localhost:3000

In the console two scripts are being printed, the two have the same attributes except for the strategy.

@albertodeago albertodeago added the bug Issue was opened via the bug report template. label Jan 26, 2022
@albertodeago
Copy link
Author

albertodeago commented Jan 26, 2022

I've digged a bit in the source code and I think this is because getPreNextScripts. Some attributes are forwarded but defer and crossorigin are overwritten, is this intentional?

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

balazsorban44 commented Jan 26, 2022

For crossOrigin, I have an answer for you:

https://nextjs.org/docs/messages/doc-crossorigin-deprecated

I'm not sure why this warning does not show up in development:

'Warning: `NextScript` attribute `crossOrigin` is deprecated. https://nextjs.org/docs/messages/doc-crossorigin-deprecated'

Although it talks about NextScript, which is not the same as next/script, but comes from next/document. I'll double-check what is happening here.

I believe defer is intentional though, but I'll double-check that as well.

@albertodeago
Copy link
Author

Oh didn't see that, thanks for the feedback (maybe the warning it's not showing up because with the Script component that code I'm not sure it's executed)

4lejandrito added a commit to 4lejandrito/next-plausible that referenced this issue Feb 18, 2022
Do not add <script> tags using next/head (see inline <script>). Use next/script instead.
See more info here: https://nextjs.org/docs/messages/no-script-tags-in-head-component

Sadly this breaks a test because of vercel/next.js#33686
@4lejandrito
Copy link

Hi!

I am also having this issue after moving from a standard <script> tag inside next/head to just next/script (see 4lejandrito/next-plausible@dc9cfad). My tests broke because of this.

Thanks!

@balazsorban44
Copy link
Member

balazsorban44 commented Feb 19, 2022

@4lejandrito do note that you don't have to put next/script inside next/head:

anywhere in their application without needing to append directly to next/head

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

See also: https://nextjs.org/docs/messages/no-script-tags-in-head-component#script-component

@4lejandrito
Copy link

4lejandrito commented Feb 21, 2022

Hi @balazsorban44,

I did remove next/head in that commit (4lejandrito/next-plausible@dc9cfad). But the crossorigin attribute is broken by next/script.

@albertodeago
Copy link
Author

@4lejandrito I don't think @balazsorban44 meant that removing from the head would have fixed the crossorigin attribute (it was just a tip, it's handy to use the script component anywhere).
He explained above that crossorigin is considered deprecated and thus ignored (https://nextjs.org/docs/messages/doc-crossorigin-deprecated)

Not sure if there are news about defer

@balazsorban44
Copy link
Member

Actually still unsure about both crossorigin and defer. The crossorigin message seems to be referring to an older or different iteration of Next.js Script.

I asked @janicklas-ralph to have a look, he might be able to provide more context here. 👍

@balazsorban44
Copy link
Member

Small update here, defer will be configurable in the next canary release. crossorigin behavior is being discussed.

@albertodeago
Copy link
Author

That's great to hear, thanks @balazsorban44

@4lejandrito
Copy link

Hi guys, any updates on the crossorigin discussion?

4lejandrito added a commit to 4lejandrito/next-plausible that referenced this issue Jun 2, 2022
Do not add <script> tags using next/head (see inline <script>). Use next/script instead.
See more info here: https://nextjs.org/docs/messages/no-script-tags-in-head-component

Sadly this breaks a test because of vercel/next.js#33686
@4lejandrito
Copy link

4lejandrito commented Jun 2, 2022

My test is still failing on next@12.1.6. The change is this.

4lejandrito added a commit to 4lejandrito/next-plausible that referenced this issue Jun 7, 2022
Do not add <script> tags using next/head (see inline <script>). Use next/script instead.
See more info here: https://nextjs.org/docs/messages/no-script-tags-in-head-component

Sadly this breaks a test because of vercel/next.js#33686
@4lejandrito
Copy link

The crossorigin attribute is working fine. It was an issue with my test suite. My bad, sorry! All my issues with this are fixed now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. Script (next/script) Related to Next.js Script Optimization.
Projects
None yet
Development

No branches or pull requests

4 participants