[ESLint] Disallow <Script /> inside _document.js & <Script /> inside the next/head component #27257

awareness481 · 2021-07-17T09:17:19Z

Feature

Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. Feature Request
Eslint unit ests added
Errors have helpful link attached, see contributing.md

Let me know if this looks good or something needs to be changed. I still need to add the error links and improve the eslint error messages.

I don't know if the CI runs the ESLint tests, but current all pass locally

…ANCH_NAME

awareness481 · 2021-07-18T08:05:59Z

This test fails because ESLint is gonna give an error but I'm not sure why _document.js is included in the test? 🤔

errors/manifest.json

errors/no-script-in-head-component.md

packages/eslint-plugin-next/lib/rules/no-script-in-document.js

packages/eslint-plugin-next/lib/rules/no-script-in-head.js

Co-authored-by: stefanprobst <stefan.probst@univie.ac.at>

…t_script_rules

errors/no-script-in-head-component.md

packages/eslint-plugin-next/lib/rules/no-script-in-head.js

Co-authored-by: Steven <steven@ceriously.com>

ijjk · 2021-08-10T03:35:43Z

Failing test suites

Commit: 7650652

test/eslint-plugin-next/no-script-in-head.test.js

no-script-in-head > invalid >
import Head from "next/head";
import Script from "next/script";

export default function Index() {
  return (
      <Head>
        <Script></Script>
      </Head>
  );
}

Expand output

● no-script-in-head › invalid ›
import Head from "next/head";
import Script from "next/script";

  export default function Index() {
    return (
        <Head>
          <Script></Script>
        </Head>
    );
  }

assert.strictEqual(received, expected)

Expected value to strictly be equal to:
  "next/script shouldn't be used inside <Head></Head>  See: https://nextjs.org/docs/messages/no-script-in-head-component "
Received:
  "next/script shouldn't be used inside next/head. See: https://nextjs.org/docs/messages/no-script-in-head-component "

Difference:

- Expected
+ Received

- next/script shouldn't be used inside <Head></Head>  See: https://nextjs.org/docs/messages/no-script-in-head-component 
+ next/script shouldn't be used inside next/head. See: https://nextjs.org/docs/messages/no-script-in-head-component

  at assertMessageMatches (../node_modules/eslint/lib/rule-tester/rule-tester.js:632:24)
  at testInvalidTemplate (../node_modules/eslint/lib/rule-tester/rule-tester.js:704:29)
  at Object.<anonymous> (../node_modules/eslint/lib/rule-tester/rule-tester.js:893:25)

errors/no-script-in-head-component.md

styfle

Great work, thanks!

awareness481 · 2021-08-13T11:32:21Z

@styfle Should I keep the PR up to date when merge conflicts occur?

styfle · 2021-08-13T22:02:47Z

Yep! If there are no merge conflicts, then the kodiak bot will merge automatically. This PR is currently position 4 so it should merge in an hour at most.

ijjk · 2021-08-13T22:54:14Z

Stats from current PR

Default Build (Decrease detected ✓)

General

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
buildDuration	15.2s	15.2s	⚠️ +16ms
buildDurationCached	3.7s	3.7s	-2ms
nodeModulesSize	49.1 MB	49.1 MB	✓

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.062	3.14	⚠️ +0.08
/ avg req/sec	816.43	796.17	⚠️ -20.26
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.941	1.922	-0.02
/error-in-render avg req/sec	1288.23	1300.62	+12.39

Client Bundles (main, webpack, commons)

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
745.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	23.1 kB	23.1 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	67 kB	67 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
polyfills-a4..dd70.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
_app-HASH.js gzip	980 B	980 B	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	329 B	329 B	✓
dynamic-HASH.js gzip	2.64 kB	2.64 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	904 B	904 B	✓
image-HASH.js gzip	4.13 kB	4.13 kB	✓
index-HASH.js gzip	261 B	261 B	✓
link-HASH.js gzip	1.66 kB	1.66 kB	✓
routerDirect..HASH.js gzip	319 B	319 B	✓
script-HASH.js gzip	387 B	387 B	✓
withRouter-HASH.js gzip	320 B	320 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	12.9 kB	12.9 kB	✓

Client Build Manifests

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
_buildManifest.js gzip	491 B	491 B	✓
Overall change	491 B	491 B	✓

Rendered Page Sizes

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
index.html gzip	541 B	541 B	✓
link.html gzip	553 B	553 B	✓
withRouter.html gzip	534 B	534 B	✓
Overall change	1.63 kB	1.63 kB	✓

Webpack 4 Mode (Decrease detected ✓)

General

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
buildDuration	12.3s	12.4s	⚠️ +186ms
buildDurationCached	5.2s	5.1s	-94ms
nodeModulesSize	49.1 MB	49.1 MB	✓

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.984	3.087	⚠️ +0.1
/ avg req/sec	837.84	809.81	⚠️ -28.03
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.871	1.944	⚠️ +0.07
/error-in-render avg req/sec	1336.03	1285.79	⚠️ -50.24

Client Bundles (main, webpack, commons)

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
16.HASH.js gzip	186 B	186 B	✓
677f882d2ed8..HASH.js gzip	14 kB	14 kB	✓
framework.HASH.js gzip	41.9 kB	41.9 kB	✓
main-HASH.js gzip	10.6 kB	10.6 kB	✓
webpack-HASH.js gzip	1.19 kB	1.19 kB	✓
Overall change	67.9 kB	67.9 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
polyfills-a4..dd70.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
_app-HASH.js gzip	965 B	965 B	✓
_error-HASH.js gzip	3.71 kB	3.71 kB	✓
amp-HASH.js gzip	552 B	552 B	✓
css-HASH.js gzip	333 B	333 B	✓
dynamic-HASH.js gzip	2.83 kB	2.83 kB	✓
head-HASH.js gzip	2.97 kB	2.97 kB	✓
hooks-HASH.js gzip	911 B	911 B	✓
index-HASH.js gzip	231 B	231 B	✓
link-HASH.js gzip	1.64 kB	1.64 kB	✓
routerDirect..HASH.js gzip	298 B	298 B	✓
script-HASH.js gzip	2.95 kB	2.95 kB	✓
withRouter-HASH.js gzip	294 B	294 B	✓
e025d2764813..52f.css gzip	125 B	125 B	✓
Overall change	17.8 kB	17.8 kB	✓

Client Build Manifests

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
_buildManifest.js gzip	497 B	497 B	✓
Overall change	497 B	497 B	✓

Rendered Page Sizes

	vercel/next.js canary	awareness481/next.js eslint_script_rules	Change
index.html gzip	585 B	585 B	✓
link.html gzip	598 B	598 B	✓
withRouter.html gzip	579 B	579 B	✓
Overall change	1.76 kB	1.76 kB	✓

Commit: 3d7e776

awareness481 added 6 commits June 19, 2021 13:54

feat(eslint): initial script rules impl.

e288f5e

Merge branch 'canary' of https://github.com/vercel/next.js into MY_BR…

6388d13

…ANCH_NAME

feat(eslint): add no-script-in-document rule & test

ed95af5

feat(eslint): add no-script-in-head rule & test

5bf91bd

fix: prettier

1a9de78

fix(eslint_script_rules): document.text & minor changes

5073500

awareness481 requested review from huozhi, ijjk, padmaia, shuding, styfle and timneutkens as code owners July 17, 2021 09:17

This comment has been minimized.

Sign in to view

feat: add error pages

7cc002e

ijjk added the type: documentation label Jul 18, 2021

fix: variable name

03e4ff4

This comment has been minimized.

Sign in to view

stefanprobst mentioned this pull request Jul 27, 2021

next/script <Script> component doesn't render if inside next/head <Head> component #27498

Closed

stefanprobst reviewed Jul 30, 2021

View reviewed changes

errors/manifest.json Outdated Show resolved Hide resolved

stefanprobst reviewed Jul 30, 2021

View reviewed changes

errors/no-script-in-head-component.md Outdated Show resolved Hide resolved

stefanprobst reviewed Jul 30, 2021

View reviewed changes

packages/eslint-plugin-next/lib/rules/no-script-in-document.js Outdated Show resolved Hide resolved

stefanprobst reviewed Jul 30, 2021

View reviewed changes

packages/eslint-plugin-next/lib/rules/no-script-in-head.js Outdated Show resolved Hide resolved

stefanprobst mentioned this pull request Jul 30, 2021

move next/script to pages/_app in script loader integration tests #27626

Merged

10 tasks

awareness481 and others added 4 commits August 7, 2021 06:06

Apply suggestions from code review

979d763

Co-authored-by: stefanprobst <stefan.probst@univie.ac.at>

Merge branch 'canary' of https://github.com/vercel/next.js into eslin…

e3cc170

…t_script_rules

fix: no-script-in-head message

73211ae

fix: lint warning

43ec629

This comment has been minimized.

Sign in to view

fix: resolve conflinct

cd9a744

This comment has been minimized.

Sign in to view

styfle requested review from housseindjirdeh and janicklas-ralph August 9, 2021 22:33

styfle reviewed Aug 9, 2021

View reviewed changes

errors/no-script-in-head-component.md Outdated Show resolved Hide resolved

styfle reviewed Aug 9, 2021

View reviewed changes

errors/no-script-in-head-component.md Outdated Show resolved Hide resolved

styfle reviewed Aug 9, 2021

View reviewed changes

packages/eslint-plugin-next/lib/rules/no-script-in-head.js Outdated Show resolved Hide resolved

Apply suggestions from code review

7650652

Co-authored-by: Steven <steven@ceriously.com>

fix: no-script-in-head unit tetst message

a0368b8

This comment has been minimized.

Sign in to view

styfle reviewed Aug 10, 2021

View reviewed changes

errors/no-script-in-head-component.md Outdated Show resolved Hide resolved

Fix title

541e41e

styfle previously approved these changes Aug 10, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

Merge branch 'canary' into eslint_script_rules

b30a937

kodiakhq bot dismissed styfle’s stale review via b30a937 August 10, 2021 18:35

This comment has been minimized.

Sign in to view

Merge branch 'canary' into eslint_script_rules

b981481

styfle approved these changes Aug 13, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

Merge branch 'canary' into eslint_script_rules

3d7e776

kodiakhq bot merged commit a28e775 into vercel:canary Aug 13, 2021

vercel locked as resolved and limited conversation to collaborators Jan 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ESLint] Disallow <Script /> inside _document.js & <Script /> inside the next/head component #27257

[ESLint] Disallow <Script /> inside _document.js & <Script /> inside the next/head component #27257

awareness481 commented Jul 17, 2021 •

edited

This comment has been minimized.

This comment has been minimized.

awareness481 commented Jul 18, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Aug 10, 2021

This comment has been minimized.

styfle left a comment

This comment has been minimized.

This comment has been minimized.

awareness481 commented Aug 13, 2021

styfle commented Aug 13, 2021

This comment has been minimized.

ijjk commented Aug 13, 2021

[ESLint] Disallow <Script /> inside _document.js & <Script /> inside the next/head component #27257

[ESLint] Disallow <Script /> inside _document.js & <Script /> inside the next/head component #27257

Conversation

awareness481 commented Jul 17, 2021 • edited

Feature

This comment has been minimized.

This comment has been minimized.

awareness481 commented Jul 18, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Aug 10, 2021

Failing test suites

This comment has been minimized.

styfle left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

awareness481 commented Aug 13, 2021

styfle commented Aug 13, 2021

This comment has been minimized.

ijjk commented Aug 13, 2021

Stats from current PR

awareness481 commented Jul 17, 2021 •

edited