-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Warn in dev mode when script tags are added with next/head #33968
Conversation
Failing test suitesCommit: ef9f1ca test/integration/amphtml-fragment-style/test/index.test.js
Expand output● AMP Fragment Styles › adds styles from fragment in AMP mode correctly
test/integration/amp-export-validation/test/index.test.js
Expand output● AMP Validation on Export › should have shown errors during build
● AMP Validation on Export › should export AMP pages
test/integration/amphtml-custom-validator/test/index.test.js
Expand output● AMP Custom Validator › should run in dev mode successfully
|
This comment has been minimized.
This comment has been minimized.
95bdcb9
to
c22c78b
Compare
This commit adds a development mode warning in the console if you try to include <script> tags in next/head, e.g. ``` <Head> <script async src="..." /> </Head> ``` The warning message explains that this pattern will not work well with Suspense/streaming and recommends using the next/script component instead.
This comment has been minimized.
This comment has been minimized.
Stats from current PRDefault BuildGeneral Overall increase
|
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
buildDuration | 20.1s | 20.2s | |
buildDurationCached | 7.6s | 7.7s | |
nodeModulesSize | 359 MB | 359 MB |
Page Load Tests Overall increase ✓
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.732 | 3.772 | |
/ avg req/sec | 669.93 | 662.84 | |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.817 | 1.803 | -0.01 |
/error-in-render avg req/sec | 1375.53 | 1386.94 | +11.41 |
Client Bundles (main, webpack, commons)
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.2 kB | 42.2 kB | ✓ |
main-HASH.js gzip | 27.3 kB | 27.3 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.2 kB | 71.2 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.37 kB | 1.37 kB | ✓ |
_error-HASH.js gzip | 194 B | 194 B | ✓ |
amp-HASH.js gzip | 312 B | 312 B | ✓ |
css-HASH.js gzip | 326 B | 326 B | ✓ |
dynamic-HASH.js gzip | 2.37 kB | 2.37 kB | ✓ |
head-HASH.js gzip | 350 B | 350 B | ✓ |
hooks-HASH.js gzip | 919 B | 919 B | ✓ |
image-HASH.js gzip | 4.94 kB | 4.94 kB | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 2.19 kB | 2.19 kB | ✓ |
routerDirect..HASH.js gzip | 321 B | 321 B | ✓ |
script-HASH.js gzip | 383 B | 383 B | ✓ |
withRouter-HASH.js gzip | 318 B | 318 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.3 kB | 14.3 kB | ✓ |
Client Build Manifests
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
_buildManifest.js gzip | 459 B | 459 B | ✓ |
Overall change | 459 B | 459 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
index.html gzip | 531 B | 531 B | ✓ |
link.html gzip | 544 B | 544 B | ✓ |
withRouter.html gzip | 527 B | 527 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | ✓ |
Diffs
Diff for main-HASH.js
@@ -3826,6 +3826,9 @@
return /*#__PURE__*/ _react.default.cloneElement(c, newProps);
}
}
+ // TODO(kara): warn for stylesheets as well as scripts
+ if (false) {
+ }
return /*#__PURE__*/ _react.default.cloneElement(c, {
key: key
});
Diff for index.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-3bcc9011ce5cf468.js"
+ src="/_next/static/chunks/main-04b00b1eea9d690e.js"
defer=""
></script>
<script
Diff for link.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-3bcc9011ce5cf468.js"
+ src="/_next/static/chunks/main-04b00b1eea9d690e.js"
defer=""
></script>
<script
Diff for withRouter.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-3bcc9011ce5cf468.js"
+ src="/_next/static/chunks/main-04b00b1eea9d690e.js"
defer=""
></script>
<script
Default Build with SWC (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
buildDuration | 24.4s | 24.1s | -305ms |
buildDurationCached | 7.7s | 7.7s | |
nodeModulesSize | 359 MB | 359 MB |
Page Load Tests Overall decrease ⚠️
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.751 | 3.709 | -0.04 |
/ avg req/sec | 666.51 | 674.03 | +7.52 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.799 | 1.846 | |
/error-in-render avg req/sec | 1389.32 | 1354.57 |
Client Bundles (main, webpack, commons)
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.3 kB | 42.3 kB | ✓ |
main-HASH.js gzip | 27.4 kB | 27.4 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.3 kB | 71.3 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
_error-HASH.js gzip | 180 B | 180 B | ✓ |
amp-HASH.js gzip | 305 B | 305 B | ✓ |
css-HASH.js gzip | 321 B | 321 B | ✓ |
dynamic-HASH.js gzip | 2.36 kB | 2.36 kB | ✓ |
head-HASH.js gzip | 342 B | 342 B | ✓ |
hooks-HASH.js gzip | 911 B | 911 B | ✓ |
image-HASH.js gzip | 4.97 kB | 4.97 kB | ✓ |
index-HASH.js gzip | 256 B | 256 B | ✓ |
link-HASH.js gzip | 2.21 kB | 2.21 kB | ✓ |
routerDirect..HASH.js gzip | 314 B | 314 B | ✓ |
script-HASH.js gzip | 375 B | 375 B | ✓ |
withRouter-HASH.js gzip | 309 B | 309 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.3 kB | 14.3 kB | ✓ |
Client Build Manifests
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
_buildManifest.js gzip | 459 B | 459 B | ✓ |
Overall change | 459 B | 459 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | kara/next.js warning | Change | |
---|---|---|---|
index.html gzip | 533 B | 533 B | ✓ |
link.html gzip | 546 B | 546 B | ✓ |
withRouter.html gzip | 527 B | 527 B | ✓ |
Overall change | 1.61 kB | 1.61 kB | ✓ |
Diffs
Diff for main-HASH.js
@@ -3826,6 +3826,9 @@
return /*#__PURE__*/ _react.default.cloneElement(c, newProps);
}
}
+ // TODO(kara): warn for stylesheets as well as scripts
+ if (false) {
+ }
return /*#__PURE__*/ _react.default.cloneElement(c, {
key: key
});
Diff for index.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-3bcc9011ce5cf468.js"
+ src="/_next/static/chunks/main-04b00b1eea9d690e.js"
defer=""
></script>
<script
Diff for link.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-3bcc9011ce5cf468.js"
+ src="/_next/static/chunks/main-04b00b1eea9d690e.js"
defer=""
></script>
<script
Diff for withRouter.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-3bcc9011ce5cf468.js"
+ src="/_next/static/chunks/main-04b00b1eea9d690e.js"
defer=""
></script>
<script
Hey @kara and everyone, not a huge deal but it looks like I'm getting a ton of these warnings in canary 6 — not because I'm adding any script tags myself, but because I'm using next-seo's JSON-LD components (which I suppose technically does indirectly add script tags, but with Are these structured data "scripts" intended to be caught by this check? Happy to whip up a quick repo to reproduce this if it would be helpful. 😊 |
In vercel#33968, a warning was added for script tags inserted through the next/head component. This change unintentionally included application/ld+json scripts, which shouldn't be triggering the warnings (as they were originally intended to catch scripts where loading order or timing could be important). This change adds an exception for application/ld+json scripts, so they do not log the warning if they are included through next/head.
Hmm, no, those types of script tags shouldn't be caught by the warning -- thanks for pointing this out! Just created a PR here to fix the issue. |
You are quick! Thanks so much!! 🚀 |
…34021) In #33968, a warning was added for script tags inserted through the next/head component. This change unintentionally included application/ld+json scripts, which shouldn't be triggering the warnings (as they were originally intended to catch scripts where loading order or timing could be important). This change adds an exception for application/ld+json scripts, so they do not log the warning if they are included through next/head.
) This commit adds a development mode warning in the console if you try to include <script> tags in next/head, e.g. ``` <Head> <script async src="..." /> </Head> ``` The warning message explains that this pattern will not work well with Suspense/streaming and recommends using the next/script component instead. TODO in follow-up PR: add same warning for stylesheets, etc ## Feature - [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [x] Integration tests added - [x] Documentation added - [x] Errors have helpful link attached, see `contributing.md`
…ercel#34021) In vercel#33968, a warning was added for script tags inserted through the next/head component. This change unintentionally included application/ld+json scripts, which shouldn't be triggering the warnings (as they were originally intended to catch scripts where loading order or timing could be important). This change adds an exception for application/ld+json scripts, so they do not log the warning if they are included through next/head.
This commit adds a development mode warning in the console
if you try to include <script> tags in next/head, e.g.
The warning message explains that this pattern will not
work well with Suspense/streaming and recommends using the
next/script component instead.
TODO in follow-up PR: add same warning for stylesheets, etc
Feature
contributing.md