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.

Sign up for GitHub

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

Jump to bottom

feat(eslint-plugin): add minimal built-in flat presets #73873

Open
wants to merge 4 commits into
base: canary
Choose a base branch
from
Open

feat(eslint-plugin): add minimal built-in flat presets #73873

wants to merge 4 commits into from
+55 −30

Conversation

SukkaW
Copy link
Contributor

@SukkaW SukkaW commented Dec 13, 2024 •
edited
Loading

Add minimal flat config presets for those who are building their own eslint flat presets and want to use their own parsers and settings.

The usage would be like:

// eslint.config.js
import eslintPluginNext from 'eslint-plugin-next';

export default [
  {
    files: [],
    plugins: {},
    settings: {},
    languageOptions: {},
    rules: {}
  },

  eslintPluginNext.flatConfigs.recommended,
  // or
  eslintPluginNext.flatConfigs.coreWebVitals,
]

@ijjk ijjk added CI approved Approve running CI for fork type: next labels Dec 13, 2024
@ijjk
Copy link
Member

ijjk commented Dec 13, 2024 •
edited
Loading

Failing test suites

Commit: 8a7d85c

__NEXT_EXPERIMENTAL_PPR=true pnpm test-dev test/e2e/socket-io/index.test.js (PPR)

  • socket-io > should support socket.io without falling back to polling
Expand output

● socket-io › should support socket.io without falling back to polling

TIMED OUT: /hello world/



undefined

  731 |
  732 |   if (hardError) {
> 733 |     throw new Error('TIMED OUT: ' + regex + '\n\n' + content + '\n\n' + lastErr)
      |           ^
  734 |   }
  735 |   return false
  736 | }

  at check (lib/next-test-utils.ts:733:11)
  at Object.<anonymous> (e2e/socket-io/index.test.js:31:5)

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

@ijjk
Copy link
Member

ijjk commented Dec 13, 2024 •
edited
Loading

Stats from current PR

Default Build (Increase detected ⚠️)
General
vercel/next.js canary SukkaW/next.js eslint-plugin-presets Change
buildDuration 19s 18.9s N/A
buildDurationCached 14.7s 12.4s N/A
nodeModulesSize 410 MB 410 MB N/A
nextStartRea..uration (ms) 473ms 480ms N/A
Client Bundles (main, webpack)
vercel/next.js canary SukkaW/next.js eslint-plugin-presets Change
1187-HASH.js gzip 50.9 kB 50.8 kB N/A
8276.HASH.js gzip 169 B 168 B N/A
8377-HASH.js gzip 5.36 kB 5.36 kB N/A
bccd1874-HASH.js gzip 53 kB 53 kB N/A
framework-HASH.js gzip 57.5 kB 57.5 kB N/A
main-app-HASH.js gzip 233 B 235 B N/A
main-HASH.js gzip 34.1 kB 34 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 0 B 0 B ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary SukkaW/next.js eslint-plugin-presets Change
polyfills-HASH.js gzip 39.4 kB 39.4 kB ✓
Overall change 39.4 kB 39.4 kB ✓
Client Pages
vercel/next.js canary SukkaW/next.js eslint-plugin-presets Change
_app-HASH.js gzip 193 B 193 B ✓
_error-HASH.js gzip 193 B 193 B ✓
amp-HASH.js gzip 512 B 510 B N/A
css-HASH.js gzip 343 B 342 B N/A
dynamic-HASH.js gzip 1.84 kB 1.84 kB ✓
edge-ssr-HASH.js gzip 265 B 265 B ✓
head-HASH.js gzip 363 B 362 B N/A
hooks-HASH.js gzip 393 B 392 B N/A
image-HASH.js gzip 4.49 kB 4.49 kB N/A
index-HASH.js gzip 268 B 268 B ✓
link-HASH.js gzip 2.35 kB 2.34 kB N/A
routerDirect..HASH.js gzip 328 B 328 B ✓
script-HASH.js gzip 397 B 397 B ✓
withRouter-HASH.js gzip 323 B 326 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B ✓
Overall change 3.59 kB 3.59 kB ✓
Client Build Manifests
vercel/next.js canary SukkaW/next.js eslint-plugin-presets Change
_buildManifest.js gzip 749 B 746 B N/A
Overall change 0 B 0 B ✓
Rendered Page Sizes
vercel/next.js canary SukkaW/next.js eslint-plugin-presets Change
index.html gzip 523 B 523 B ✓
link.html gzip 538 B 537 B N/A
withRouter.html gzip 518 B 521 B N/A
Overall change 523 B 523 B ✓
Edge SSR bundle Size
vercel/next.js canary SukkaW/next.js eslint-plugin-presets Change
edge-ssr.js gzip 128 kB 128 kB N/A
page.js gzip 204 kB 203 kB N/A
Overall change 0 B 0 B ✓
Middleware size
vercel/next.js canary SukkaW/next.js eslint-plugin-presets Change
middleware-b..fest.js gzip 668 B 667 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 31.2 kB 31.2 kB N/A
edge-runtime..pack.js gzip 844 B 844 B ✓
Overall change 844 B 844 B ✓
Next Runtimes
vercel/next.js canary SukkaW/next.js eslint-plugin-presets Change
523-experime...dev.js gzip 322 B 322 B ✓
523.runtime.dev.js gzip 314 B 314 B ✓
app-page-exp...dev.js gzip 323 kB 323 kB N/A
app-page-exp..prod.js gzip 127 kB 127 kB N/A
app-page-tur..prod.js gzip 140 kB 140 kB N/A
app-page-tur..prod.js gzip 135 kB 135 kB N/A
app-page.run...dev.js gzip 313 kB 313 kB N/A
app-page.run..prod.js gzip 123 kB 123 kB N/A
app-route-ex...dev.js gzip 37.4 kB 37.3 kB N/A
app-route-ex..prod.js gzip 25.5 kB 25.3 kB N/A
app-route-tu..prod.js gzip 25.5 kB 25.4 kB N/A
app-route-tu..prod.js gzip 25.3 kB 25.2 kB N/A
app-route.ru...dev.js gzip 39 kB 38.9 kB N/A
app-route.ru..prod.js gzip 25.3 kB 25.2 kB N/A
pages-api-tu..prod.js gzip 9.69 kB 9.67 kB N/A
pages-api.ru...dev.js gzip 11.6 kB 11.6 kB N/A
pages-api.ru..prod.js gzip 9.68 kB 9.67 kB N/A
pages-turbo...prod.js gzip 21.7 kB 21.7 kB N/A
pages.runtim...dev.js gzip 27.4 kB 27.4 kB N/A
pages.runtim..prod.js gzip 21.7 kB 21.7 kB N/A
server.runti..prod.js gzip 916 kB 916 kB N/A
Overall change 636 B 636 B ✓
build cache Overall increase ⚠️
vercel/next.js canary SukkaW/next.js eslint-plugin-presets Change
0.pack gzip 2.05 MB 2.05 MB N/A
index.pack gzip 72.3 kB 73.4 kB ⚠️ +1.07 kB
Overall change 72.3 kB 73.4 kB ⚠️ +1.07 kB
Diff details
Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for 1187-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js 
failed to diff
Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js 
failed to diff
Diff for app-page.runtime.prod.js

Diff too large to display

Diff for app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Diff for pages-api-tu..time.prod.js

Diff too large to display

Diff for pages-api.runtime.dev.js

Diff too large to display

Diff for pages-api.ru..time.prod.js

Diff too large to display

Diff for pages-turbo...time.prod.js

Diff too large to display

Diff for pages.runtime.dev.js

Diff too large to display

Diff for pages.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: 8a7d85c

@SukkaW SukkaW marked this pull request as draft December 13, 2024 02:09
@SukkaW SukkaW marked this pull request as ready for review December 13, 2024 02:26
@darthmaim
Copy link
Contributor

Take a look at these two comments and the surrounding discussion and also at the ESLint docs:

TLDR: ESLint prefers not to use a flatConfigs export, but instead export all configs from .configs, and suffix old eslintrc-style configs with -legacy.

@SukkaW
Copy link
Contributor Author

SukkaW commented Dec 13, 2024 •
edited
Loading

TLDR: ESLint prefers not to use a flatConfigs export, but instead export all configs from .configs, and suffix old eslintrc-style configs with -legacy.

Adding a -legacy suffix means there will be a breaking change, which means we need to wait for another breaking change. Are you willing to wait for another year?

In the meantime, attaching an extra type-safe flat preset under the configs is impossible without triggering a TypeScript error. Self-reference also doesn't work, you will get a "used-before-declare" error.

While plugin.flatConfigs is not recommended, you can import anything into the eslint.config.js, so why bother?

@darthmaim
Copy link
Contributor

Yeah I agree, just wanted to make sure that this is considered and that there is already a plan in place to match the recommendation in the next major next.js release.

Ideally the flat config would have been part of 15.0 along with eslint 9 support, sadly eslint 9 support landed with such short notice before 15.0 that no one catched this in canary :(

rakleed
rakleed reviewed Dec 14, 2024
View reviewed changes
packages/eslint-plugin-next/src/index.ts Outdated
module.exports = plugin
module.exports.flatConfig = {
recommended: {
name: 'eslint-plugin-next recommended',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably better to call it next/recommended, like other plugins do?

Same for core-web-vitals config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 8a7d85c (#73873).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rakleed rakleed rakleed left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
CI approved Approve running CI for fork type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SukkaW @ijjk @darthmaim @rakleed