Adds linting rule to avoid assignment to 'module' variable #35279

mward-sudo · 2022-03-13T03:06:29Z

Fixes #34859

Bug

Related issues linked using fixes #number
Integration tests added
Errors have helpful link attached, see contributing.md

Documentation / Examples

Make sure the linting passes by running yarn lint

mward-sudo · 2022-03-13T03:08:42Z

Hi, first contribution. Happy to make any suggested changes.

packages/eslint-plugin-next/lib/rules/no-assign-module-variable.js

mward-sudo · 2022-03-15T16:36:46Z

Not sure why the actions are failing :/

ijjk · 2022-03-15T17:52:59Z

Failing test suites

Commit: b2ddbc1

yarn testheadless test/integration/eslint/test/index.test.js

ESLint > Next Build > empty directories do not fail the build
ESLint > Next Build > eslint ignored directories do not fail the build
ESLint > Next Build > eslint caching is enabled
ESLint > Next Build > eslint cache lives in the user defined build directory
ESLint > Next Lint > success message when no warnings or errors
ESLint > Next Lint > max warnings flag does not error when warnings do not exceed threshold
ESLint > Next Lint > format flag supports additional user-defined formats
ESLint > Next Lint > eslint caching is enabled by default
ESLint > Next Lint > eslint caching is disabled with the --no-cache flag
ESLint > Next Lint > the default eslint cache lives in the user defined build directory
ESLint > Next Lint > the --cache-location flag allows the user to define a separate cache location
ESLint > Next Lint > the default eslint caching strategy is metadata
ESLint > Next Lint > cache with content strategy is different from the one with default strategy

Expand output

● ESLint › Next Build › empty directories do not fail the build

expect(received).toContain(expected) // indexOf

Expected substring: "Compiled successfully"
Received string:    "info  - Checking validity of types...·
Failed to compile.·
./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable
4:5  Warning: External synchronous scripts are forbidden. See: https://nextjs.org/docs/messages/no-sync-scripts  @next/next/no-sync-scripts·
info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules
"

  124 |         'Warning: External synchronous scripts are forbidden'
  125 |       )
> 126 |       expect(output).toContain('Compiled successfully')
      |                      ^
  127 |     })
  128 |
  129 |     test('eslint ignored directories do not fail the build', async () => {

  at Object.<anonymous> (integration/eslint/test/index.test.js:126:22)

● ESLint › Next Build › eslint ignored directories do not fail the build

expect(received).toContain(expected) // indexOf

Expected substring: "Compiled successfully"
Received string:    "info  - Checking validity of types...·
Failed to compile.·
./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable
4:5  Warning: External synchronous scripts are forbidden. See: https://nextjs.org/docs/messages/no-sync-scripts  @next/next/no-sync-scripts·
info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules
"

  139 |         'Warning: External synchronous scripts are forbidden'
  140 |       )
> 141 |       expect(output).toContain('Compiled successfully')
      |                      ^
  142 |     })
  143 |
  144 |     test('missing Next.js plugin', async () => {

  at Object.<anonymous> (integration/eslint/test/index.test.js:141:22)

● ESLint › Next Build › eslint caching is enabled

command failed with code 1
warn  - No build cache found. Please configure build caching for faster rebuilds. Read more: https://nextjs.org/docs/messages/no-cache
info  - Checking validity of types...

Failed to compile.

./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable

info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules

  199 |       ) {
  200 |         return reject(
> 201 |           new Error(`command failed with code ${code}\n${mergedStdio}`)
      |           ^
  202 |         )
  203 |       }
  204 |

  at ChildProcess.<anonymous> (lib/next-test-utils.js:201:11)

● ESLint › Next Build › eslint cache lives in the user defined build directory

command failed with code 1
warn  - No build cache found. Please configure build caching for faster rebuilds. Read more: https://nextjs.org/docs/messages/no-cache
info  - Checking validity of types...

Failed to compile.

./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable

info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules

  199 |       ) {
  200 |         return reject(
> 201 |           new Error(`command failed with code ${code}\n${mergedStdio}`)
      |           ^
  202 |         )
  203 |       }
  204 |

  at ChildProcess.<anonymous> (lib/next-test-utils.js:201:11)

● ESLint › Next Lint › success message when no warnings or errors

expect(received).toContain(expected) // indexOf

Expected substring: "No ESLint warnings or errors"
Received string:    "
./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable·
info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules
"

  341 |
  342 |       const output = stdout + stderr
> 343 |       expect(output).toContain('No ESLint warnings or errors')
      |                      ^
  344 |     })
  345 |
  346 |     test("don't create .eslintrc file if package.json has eslintConfig field", async () => {

  at Object.<anonymous> (integration/eslint/test/index.test.js:343:22)

● ESLint › Next Lint › max warnings flag does not error when warnings do not exceed threshold

expect(received).toEqual(expected) // deep equality

Expected: ""
Received: "
./pages/about.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable
4:5  Warning: External synchronous scripts are forbidden. See: https://nextjs.org/docs/messages/no-sync-scripts  @next/next/no-sync-scripts·
./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable
4:5  Warning: External synchronous scripts are forbidden. See: https://nextjs.org/docs/messages/no-sync-scripts  @next/next/no-sync-scripts·
info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules
"

  441 |       )
  442 |
> 443 |       expect(stderr).toEqual('')
      |                      ^
  444 |       expect(stderr).not.toContain(
  445 |         'Warning: External synchronous scripts are forbidden'
  446 |       )

  at Object.<anonymous> (integration/eslint/test/index.test.js:443:22)

● ESLint › Next Lint › format flag supports additional user-defined formats

expect(received).toContain(expected) // indexOf

Expected substring: "<script src=\"https://example.com\" />"
Received string:    ""

  464 |         'warning: External synchronous scripts are forbidden'
  465 |       )
> 466 |       expect(stdout).toContain('<script src="https://example.com" />')
      |                      ^
  467 |       expect(stdout).toContain('2 warnings found')
  468 |     })
  469 |

  at Object.<anonymous> (integration/eslint/test/index.test.js:466:22)

● ESLint › Next Lint › eslint caching is enabled by default

command failed with code 1

./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable

info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules

  199 |       ) {
  200 |         return reject(
> 201 |           new Error(`command failed with code ${code}\n${mergedStdio}`)
      |           ^
  202 |         )
  203 |       }
  204 |

  at ChildProcess.<anonymous> (lib/next-test-utils.js:201:11)

● ESLint › Next Lint › eslint caching is disabled with the --no-cache flag

command failed with code 1

./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable

info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules

  199 |       ) {
  200 |         return reject(
> 201 |           new Error(`command failed with code ${code}\n${mergedStdio}`)
      |           ^
  202 |         )
  203 |       }
  204 |

  at ChildProcess.<anonymous> (lib/next-test-utils.js:201:11)

● ESLint › Next Lint › the default eslint cache lives in the user defined build directory

command failed with code 1

./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable

info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules

  199 |       ) {
  200 |         return reject(
> 201 |           new Error(`command failed with code ${code}\n${mergedStdio}`)
      |           ^
  202 |         )
  203 |       }
  204 |

  at ChildProcess.<anonymous> (lib/next-test-utils.js:201:11)

● ESLint › Next Lint › the --cache-location flag allows the user to define a separate cache location

command failed with code 1

./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable

info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules

  199 |       ) {
  200 |         return reject(
> 201 |           new Error(`command failed with code ${code}\n${mergedStdio}`)
      |           ^
  202 |         )
  203 |       }
  204 |

  at ChildProcess.<anonymous> (lib/next-test-utils.js:201:11)

● ESLint › Next Lint › the default eslint caching strategy is metadata

command failed with code 1

./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable

info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules

  199 |       ) {
  200 |         return reject(
> 201 |           new Error(`command failed with code ${code}\n${mergedStdio}`)
      |           ^
  202 |         )
  203 |       }
  204 |

  at ChildProcess.<anonymous> (lib/next-test-utils.js:201:11)

● ESLint › Next Lint › cache with content strategy is different from the one with default strategy

command failed with code 1

./pages/index.js
1:1  Error: Definition for rule '@next/next/no-assign-module-variable' was not found.  @next/next/no-assign-module-variable

info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules

  199 |       ) {
  200 |         return reject(
> 201 |           new Error(`command failed with code ${code}\n${mergedStdio}`)
      |           ^
  202 |         )
  203 |       }
  204 |

  at ChildProcess.<anonymous> (lib/next-test-utils.js:201:11)

Read more about building and testing Next.js in contributing.md.

yarn testheadless test/integration/telemetry/test/index.test.js

Telemetry CLI > emits telemetry for lint during build
Telemetry CLI > emits telemetry for usage of swc

Expand output

● Telemetry CLI › emits telemetry for lint during build

TypeError: Cannot read property 'pop' of null

  546 |
  547 |     const event2 = /NEXT_BUILD_FEATURE_USAGE[\s\S]+?{([\s\S]+?)}/
> 548 |       .exec(stderr)
      |                   ^
  549 |       .pop()
  550 |     expect(event2).toContain(`"featureName": "build-lint"`)
  551 |     expect(event2).toContain(`"invocationCount": 1`)

  at Object.<anonymous> (integration/telemetry/test/index.test.js:548:19)

● Telemetry CLI › emits telemetry for usage of swc

ENOENT: no such file or directory, rename '/home/runner/work/next.js/next.js/test/integration/telemetry/next.config.swc' -> '/home/runner/work/next.js/next.js/test/integration/telemetry/next.config.js'

Read more about building and testing Next.js in contributing.md.

packages/eslint-plugin-next/lib/index.js

balazsorban44 · 2022-03-16T10:37:29Z

Not sure why the actions are failing :/

It looks as if something is currently failing in the tests on canary branch.

As a first-time contributor, we just have to manually approve CI runs for you, no problem. 👍

mward-sudo · 2022-03-18T16:53:49Z

Rebased and squashed commits.

mward-sudo · 2022-04-08T23:02:57Z

Failing test suites

Commit: b2ddbc1

yarn testheadless test/integration/eslint/test/index.test.js

ESLint > Next Build > empty directories do not fail the build

ESLint > Next Build > eslint ignored directories do not fail the build

ESLint > Next Build > eslint caching is enabled

ESLint > Next Build > eslint cache lives in the user defined build directory

ESLint > Next Lint > success message when no warnings or errors

ESLint > Next Lint > max warnings flag does not error when warnings do not exceed threshold

ESLint > Next Lint > format flag supports additional user-defined formats

ESLint > Next Lint > eslint caching is enabled by default

ESLint > Next Lint > eslint caching is disabled with the --no-cache flag

ESLint > Next Lint > the default eslint cache lives in the user defined build directory

ESLint > Next Lint > the --cache-location flag allows the user to define a separate cache location

ESLint > Next Lint > the default eslint caching strategy is metadata

ESLint > Next Lint > cache with content strategy is different from the one with default strategy

Expand output
Read more about building and testing Next.js in contributing.md.

yarn testheadless test/integration/telemetry/test/index.test.js

Telemetry CLI > emits telemetry for lint during build

Telemetry CLI > emits telemetry for usage of swc

Expand output
Read more about building and testing Next.js in contributing.md.

@ijjk these tests are all passing now.

ijjk · 2022-04-11T10:19:14Z

Stats from current PR

Default Build (Decrease detected ✓)

General

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
buildDuration	15.5s	15.2s	-361ms
buildDurationCached	6.1s	6s	-65ms
nodeModulesSize	478 MB	478 MB	✓

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.001	3.034	⚠️ +0.03
/ avg req/sec	833.17	823.92	⚠️ -9.25
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.176	1.173	0
/error-in-render avg req/sec	2125.05	2131.68	+6.63

Client Bundles (main, webpack)

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
925.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42 kB	42 kB	✓
main-HASH.js gzip	28.3 kB	28.3 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	72 kB	72 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
_app-HASH.js gzip	1.36 kB	1.36 kB	✓
_error-HASH.js gzip	192 B	192 B	✓
amp-HASH.js gzip	309 B	309 B	✓
css-HASH.js gzip	327 B	327 B	✓
dynamic-HASH.js gzip	3.05 kB	3.05 kB	✓
head-HASH.js gzip	351 B	351 B	✓
hooks-HASH.js gzip	920 B	920 B	✓
image-HASH.js gzip	5.73 kB	5.73 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	2.36 kB	2.36 kB	✓
routerDirect..HASH.js gzip	320 B	320 B	✓
script-HASH.js gzip	392 B	392 B	✓
withRouter-HASH.js gzip	319 B	319 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	16 kB	16 kB	✓

Client Build Manifests

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
_buildManifest.js gzip	460 B	460 B	✓
Overall change	460 B	460 B	✓

Rendered Page Sizes

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
index.html gzip	533 B	533 B	✓
link.html gzip	546 B	546 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Default Build with SWC (Decrease detected ✓)

General

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
buildDuration	18.3s	18.1s	-220ms
buildDurationCached	6s	6s	-63ms
nodeModulesSize	478 MB	478 MB	✓

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.045	3.039	-0.01
/ avg req/sec	821	822.74	+1.74
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.173	1.177	0
/error-in-render avg req/sec	2131.93	2123.33	⚠️ -8.6

Client Bundles (main, webpack)

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
925.HASH.js gzip	178 B	178 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	28.7 kB	28.7 kB	✓
webpack-HASH.js gzip	1.45 kB	1.45 kB	✓
Overall change	72.6 kB	72.6 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
_app-HASH.js gzip	1.35 kB	1.35 kB	✓
_error-HASH.js gzip	179 B	179 B	✓
amp-HASH.js gzip	313 B	313 B	✓
css-HASH.js gzip	325 B	325 B	✓
dynamic-HASH.js gzip	3.03 kB	3.03 kB	✓
head-HASH.js gzip	351 B	351 B	✓
hooks-HASH.js gzip	921 B	921 B	✓
image-HASH.js gzip	5.77 kB	5.77 kB	✓
index-HASH.js gzip	261 B	261 B	✓
link-HASH.js gzip	2.44 kB	2.44 kB	✓
routerDirect..HASH.js gzip	322 B	322 B	✓
script-HASH.js gzip	393 B	393 B	✓
withRouter-HASH.js gzip	317 B	317 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	16.1 kB	16.1 kB	✓

Client Build Manifests

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
_buildManifest.js gzip	458 B	458 B	✓
Overall change	458 B	458 B	✓

Rendered Page Sizes

	vercel/next.js canary	scripthungry/next.js lint-module-variable	Change
index.html gzip	531 B	531 B	✓
link.html gzip	545 B	545 B	✓
withRouter.html gzip	527 B	527 B	✓
Overall change	1.6 kB	1.6 kB	✓

Commit: f4ff456

timneutkens · 2022-04-15T12:46:06Z

Thanks @mward-sudo 🎉

mward-sudo requested review from timneutkens, ijjk, shuding and huozhi as code owners March 13, 2022 03:06

ijjk added area: documentation type: next labels Mar 13, 2022

balazsorban44 changed the title ~~Adds linting rule to avoid assignment to 'module' variable. Fixes #34859~~ Adds linting rule to avoid assignment to 'module' variable Mar 15, 2022

balazsorban44 reviewed Mar 15, 2022

View reviewed changes

packages/eslint-plugin-next/lib/rules/no-assign-module-variable.js Outdated Show resolved Hide resolved

balazsorban44 previously approved these changes Mar 15, 2022

View reviewed changes

mward-sudo dismissed balazsorban44’s stale review via 3b19d1e March 15, 2022 15:55

balazsorban44 reviewed Mar 15, 2022

View reviewed changes

packages/eslint-plugin-next/lib/index.js Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Adds linting rule to avoid assignment to 'module' variable. Fixes #34859

f4ff456

balazsorban44 approved these changes Apr 13, 2022

View reviewed changes

timneutkens approved these changes Apr 15, 2022

View reviewed changes

timneutkens merged commit 1be5f36 into vercel:canary Apr 15, 2022

github-actions bot locked as resolved and limited conversation to collaborators May 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds linting rule to avoid assignment to 'module' variable #35279

Adds linting rule to avoid assignment to 'module' variable #35279

mward-sudo commented Mar 13, 2022 •

edited by balazsorban44

mward-sudo commented Mar 13, 2022

mward-sudo commented Mar 15, 2022

ijjk commented Mar 15, 2022 •

edited

This comment has been minimized.

balazsorban44 commented Mar 16, 2022

This comment has been minimized.

mward-sudo commented Mar 18, 2022 •

edited

mward-sudo commented Apr 8, 2022

Failing test suites

ijjk commented Apr 11, 2022

timneutkens commented Apr 15, 2022

Adds linting rule to avoid assignment to 'module' variable #35279

Adds linting rule to avoid assignment to 'module' variable #35279

Conversation

mward-sudo commented Mar 13, 2022 • edited by balazsorban44

Bug

Documentation / Examples

mward-sudo commented Mar 13, 2022

mward-sudo commented Mar 15, 2022

ijjk commented Mar 15, 2022 • edited

Failing test suites

This comment has been minimized.

balazsorban44 commented Mar 16, 2022

This comment has been minimized.

mward-sudo commented Mar 18, 2022 • edited

mward-sudo commented Apr 8, 2022

Failing test suites

ijjk commented Apr 11, 2022

Stats from current PR

timneutkens commented Apr 15, 2022

mward-sudo commented Mar 13, 2022 •

edited by balazsorban44

ijjk commented Mar 15, 2022 •

edited

mward-sudo commented Mar 18, 2022 •

edited