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

Eslint Migration - Nextjs #2935

Closed
1 task done
mouadbelbah-planoly opened this issue May 21, 2024 · 17 comments
Closed
1 task done

Eslint Migration - Nextjs #2935

mouadbelbah-planoly opened this issue May 21, 2024 · 17 comments
Labels
S-Needs repro Status: needs a reproduction

Comments

@mouadbelbah-planoly
Copy link

Environment information

CLI:
  Version:                      1.7.3
  Color support:                true

Platform:
  CPU Architecture:             aarch64
  OS:                           macos

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v20.13.1"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "npm/10.5.2"

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 false

Workspace:
  Open Documents:               0

What happened?

  1. Started a new Nextjs Project with Eslint
  2. Installed biome following the get-started guide.
  3. ran npx biome migrate eslint --write

Received Error:

Migration has encountered an error: The module '..../node_modules/eslint-config-next/index.js' cannot be loaded. Make sure that the module exists.

Expected result

It should migrate Nextjs Eslint config

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@ematipico ematipico added the S-Needs repro Status: needs a reproduction label May 21, 2024
@ematipico
Copy link
Member

Please provide us with a minimal reproduction

@Conaclos
Copy link
Member

Some idea: If you use a package manager that doesn't make accessible indirect dependencies, then Biome cam not be able to load these dependencies. Anyway we need more info to understand your issue.

@chribjel
Copy link

chribjel commented May 22, 2024

the repro is very simple:

npm create next-app # Yes to everthing
cd my-app # Or what you called your app
npm install --save-dev --save-exact @biomejs/biome
npx @biomejs/biome init
npx @biomejs/biome migrate eslint --write

@Conaclos
Copy link
Member

The reported error is not so informative and even misleading. I will open a PR to fix that.

With a proper error propagation I am getting the following error:

  ✖ Migration has encountered an error: `node` was invoked to resolve '/playroom/my-app/node_modules/eslint-config-next/index.js'. This invocation failed with the following error:
    /playroom/my-app/node_modules/@rushstack/eslint-patch/lib/_patch-base.js:167
                throw new Error('Failed to patch ESLint because the calling module was not recognized.\n' +
                      ^
    
    Error: Failed to patch ESLint because the calling module was not recognized.
    If you are using a newer ESLint version that may be unsupported, please create a GitHub issue:
    https://github.com/microsoft/rushstack/issues
        at Object.<anonymous> (/playroom/my-app/node_modules/@rushstack/eslint-patch/lib/_patch-base.js:167:19)
        at Module._compile (node:internal/modules/cjs/loader:1455:14)
        at Module._extensions..js (node:internal/modules/cjs/loader:1534:10)
        at Module.load (node:internal/modules/cjs/loader:1265:32)
        at Module._load (node:internal/modules/cjs/loader:1081:12)
        at Module.require (node:internal/modules/cjs/loader:1290:19)
        at require (node:internal/modules/helpers:188:18)
        at Object.<anonymous> (/playroom/my-app/node_modules/@rushstack/eslint-patch/lib/modern-module-resolution.js:11:23)
        at Module._compile (node:internal/modules/cjs/loader:1455:14)
        at Module._extensions..js (node:internal/modules/cjs/loader:1534:10)
    
    Node.js v22.0.0

Under the hood Biome executes the ESLint configuration files.
The problem may come from using different ESLint version in the dependency tree.
This is clearly not a Biome issue.

@ematipico ematipico closed this as not planned Won't fix, can't repro, duplicate, stale May 22, 2024
@chribjel
Copy link

Even though it is not a Biome issue, it is an issue with the migration tool, no? Maybe I am misunderstanding.

The ESLint config that comes with Next works, thus the migration tool, if perfect, should be able to migrate it(?)

I just want what is best for adoption of Biome. Next is a really popular framework and i believe that if the migration tool works for Next, the adoption would increase a lot.

@ematipico
Copy link
Member

Our migration tool is a best effort tool.

While we try to do our best to accommodate possible bugs on our end, bugs that could come from other plugins can't be solved on our end.

@marmotz
Copy link

marmotz commented May 27, 2024

Ok, and how can we migrate manually ?

@Conaclos
Copy link
Member

Conaclos commented May 27, 2024

The issue seems to be the same as microsoft/rushstack#4301

Biome uses node to run the file. If you execute the following command, you will get the same error.

node ./node_modules/eslint-config-next/index.js

I think that ESLint encounters the issue, but silence this?

Ok, and how can we migrate manually ?

Reading the stack trace, you could just remove the line that imports @rushstack/eslint-patch/modern-module-resolution in ./node_modules/eslint-config-next/index.js and executes biome migrate sslint. This results in the following config:

{
	"linter": {
		"enabled": true,
		"rules": {
			"recommended": false,
			"a11y": {
				"noAriaUnsupportedElements": "warn",
				"noBlankTarget": "off",
				"useAltText": "warn",
				"useAriaPropsForRole": "warn",
				"useValidAriaProps": "warn",
				"useValidAriaValues": "warn"
			},
			"correctness": {
				"noChildrenProp": "error",
				"useExhaustiveDependencies": "warn",
				"useHookAtTopLevel": "error",
				"useJsxKeyInIterable": "error"
			},
			"security": { "noDangerouslySetInnerHtmlWithChildren": "error" },
			"suspicious": { "noCommentText": "error", "noDuplicateJsxProps": "error" }
		}
	}
}

@anandabhi04
Copy link

The reported error is not so informative and even misleading. I will open a PR to fix that.

With a proper error propagation I am getting the following error:

  ✖ Migration has encountered an error: `node` was invoked to resolve '/playroom/my-app/node_modules/eslint-config-next/index.js'. This invocation failed with the following error:
    /playroom/my-app/node_modules/@rushstack/eslint-patch/lib/_patch-base.js:167
                throw new Error('Failed to patch ESLint because the calling module was not recognized.\n' +
                      ^
    
    Error: Failed to patch ESLint because the calling module was not recognized.
    If you are using a newer ESLint version that may be unsupported, please create a GitHub issue:
    https://github.com/microsoft/rushstack/issues
        at Object.<anonymous> (/playroom/my-app/node_modules/@rushstack/eslint-patch/lib/_patch-base.js:167:19)
        at Module._compile (node:internal/modules/cjs/loader:1455:14)
        at Module._extensions..js (node:internal/modules/cjs/loader:1534:10)
        at Module.load (node:internal/modules/cjs/loader:1265:32)
        at Module._load (node:internal/modules/cjs/loader:1081:12)
        at Module.require (node:internal/modules/cjs/loader:1290:19)
        at require (node:internal/modules/helpers:188:18)
        at Object.<anonymous> (/playroom/my-app/node_modules/@rushstack/eslint-patch/lib/modern-module-resolution.js:11:23)
        at Module._compile (node:internal/modules/cjs/loader:1455:14)
        at Module._extensions..js (node:internal/modules/cjs/loader:1534:10)
    
    Node.js v22.0.0

Under the hood Biome executes the ESLint configuration files. The problem may come from using different ESLint version in the dependency tree. This is clearly not a Biome issue.

I am having the same issue how can i fix it.

@anandabhi04
Copy link

I don't have ./node_modules/eslint-config-next/index.js in my repo

@Conaclos
Copy link
Member

I am having the same issue how can i fix it.
I don't have ./node_modules/eslint-config-next/index.js in my repo

Could you open a new issue with detailed information? It is hard to help without any information.

@aksh1618
Copy link

Reading the stack trace, you could just remove the line that imports @rushstack/eslint-patch/modern-module-resolution in ./node_modules/eslint-config-next/index.js and executes biome migrate sslint. This results in the following config:

It might be worthwhile adding this as a note in the migration doc, as this fix is likely applicable for any given NextJS project.

@estevan-ulian
Copy link

The issue seems to be the same as microsoft/rushstack#4301

Biome uses node to run the file. If you execute the following command, you will get the same error.

node ./node_modules/eslint-config-next/index.js

I think that ESLint encounters the issue, but silence this?

Ok, and how can we migrate manually ?

Reading the stack trace, you could just remove the line that imports @rushstack/eslint-patch/modern-module-resolution in ./node_modules/eslint-config-next/index.js and executes biome migrate sslint. This results in the following config:

{
	"linter": {
		"enabled": true,
		"rules": {
			"recommended": false,
			"a11y": {
				"noAriaUnsupportedElements": "warn",
				"noBlankTarget": "off",
				"useAltText": "warn",
				"useAriaPropsForRole": "warn",
				"useValidAriaProps": "warn",
				"useValidAriaValues": "warn"
			},
			"correctness": {
				"noChildrenProp": "error",
				"useExhaustiveDependencies": "warn",
				"useHookAtTopLevel": "error",
				"useJsxKeyInIterable": "error"
			},
			"security": { "noDangerouslySetInnerHtmlWithChildren": "error" },
			"suspicious": { "noCommentText": "error", "noDuplicateJsxProps": "error" }
		}
	}
}

These steps work for me, but using the following command:

npx @biomejs/biome migrate eslint --write

I'm using next 15.0.1

@Conaclos
Copy link
Member

Conaclos commented Nov 14, 2024

A possible solution could be using a NodeJs module hook in order to skip the loading of @rushstack/eslint-patch/modern-module-resolution. This could require Biome to create a temporary file that contains the hooks. Otherwise, we could suggest emitting a warning instead of an hard error when @rushstack/eslint-patch is not able to detect ESLint.

@maronnjapan
Copy link

Thank you for checking and addressing the issue (#4528) I posted last week.
I wanted to test the fixes, so I compiled the Rust files from the main branch and integrated them into my project, then ran the migration command.
However, the same error occurred and I was unable to migrate.
If I want to test the functionality, should I wait for a release that includes the fixed code rather than replacing it with compiled files?

@Conaclos
Copy link
Member

@maronnjapan I just released a new nightly version of Biome. You can use it for testing the fixes :)

@maronnjapan
Copy link

@Conaclos
Thank you for releasing the nightly version.
I tried installing it and running the migration command, and the "ERR_MODULE_NOT_FOUND" error no longer occurs!
Thank you very much !!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-Needs repro Status: needs a reproduction
Projects
None yet
Development

No branches or pull requests

9 participants