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

Update next/script docs to clear up confusion around next/head and client-side JS #26253

Merged
merged 1 commit into from Jun 17, 2021

Conversation

timneutkens
Copy link
Member

Bug

  • Related issues linked using fixes #number
  • Integration tests added

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • Integration tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.

Documentation / Examples

Fixes #26187
Fixes #26158

  • Make sure the linting passes

@timneutkens timneutkens merged commit 72743bc into vercel:canary Jun 17, 2021
@timneutkens timneutkens deleted the fix/update-script-doc-more branch June 17, 2021 13:46
@Vadorequest
Copy link
Contributor

The doc has changed and I understand better the issue now, but the eslint rule might need to be reviewed, too.

@timneutkens
Copy link
Member Author

timneutkens commented Jun 17, 2021

The doc has changed and I understand better the issue now, but the eslint rule might need to be reviewed, too.

Can you share what part exactly? Happy to have a look!

@Vadorequest
Copy link
Contributor

@timneutkens If you add a script tag within a NextJS Head element, it'll warn.

image

Considering the documentation update, this behavior is likely to push devs to use Script there instead, although it shouldn't be used there.

@timneutkens
Copy link
Member Author

In the case that you're showing you should use <Script> with the right strategy 🤔

@Vadorequest
Copy link
Contributor

Vadorequest commented Jun 18, 2021

If I do that, I ran into the Cannot read property 'tagName' of null error.

NextHead (line 91) is next/head component, maybe that wasn't clear.

@timneutkens
Copy link
Member Author

The docs linked show to not wrap next/head right 🤔

@Vadorequest
Copy link
Contributor

Yes, the doc say not to use Script component when wrapping with next/head. I'm wrapping with next/head, so I shouldn't use the Script component, but ESLint will complain if I don't.

@timneutkens
Copy link
Member Author

timneutkens commented Jun 20, 2021

This docs update means don't put it inside of next/head not "if you're using <script> in next/head". In all cases next/script is recommended.

flybayer pushed a commit to blitz-js/next.js that referenced this pull request Jun 24, 2021
@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants