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

🐛 Unable to migrate ESLint configuration that extends @antfu/eslint-config #4802

Open
1 task done
Kamillaova opened this issue Dec 29, 2024 · 18 comments · May be fixed by #5255
Open
1 task done

🐛 Unable to migrate ESLint configuration that extends @antfu/eslint-config #4802

Kamillaova opened this issue Dec 29, 2024 · 18 comments · May be fixed by #5255
Labels
S-Needs triage Status: this issue needs to be triaged

Comments

@Kamillaova
Copy link

Kamillaova commented Dec 29, 2024

Environment information

CLI:
  Version:                      1.9.4
  Color support:                true

Platform:
  CPU Architecture:             aarch64
  OS:                           macos

Environment:
  BIOME_LOG_PATH:               unset
  BIOME_LOG_PREFIX_NAME:        unset
  BIOME_CONFIG_PATH:            unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v22.8.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         unset

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. git clome https://github.com/Kamillaova/biome-eslint-migration-error-mre
  2. direnv allow # or skip
  3. pnpm i --frozen-lockfile
  4. biome migrate eslint --write # or node_modules/.bin/biome without direnv
./eslint.config.js:1:1 deserialize ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Incorrect type, expected an array, but received an object.

  > 1 │ {"_operations":[null],"_operationsOverrides":[],"_operationsResolved":[],"_renames":{"@eslint-react":"react","@eslint-react/dom":"react-dom","@eslint-react/hooks-extra":"react-hooks-extra","@eslint-react/naming-convention":"react-naming-convention","@stylistic":"style","@typescript-eslint":"ts","import-x":"import","n":"node","vitest":"test","yml":"yaml"},"_pluginsConflictsError":{}}
      │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    2 │


migrate ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Migration has encountered an error: Could not deserialize the Eslint configuration file

Expected result

Successful migration

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@Kamillaova Kamillaova added the S-Needs triage Status: this issue needs to be triaged label Dec 29, 2024
@pnodet
Copy link
Contributor

pnodet commented Mar 2, 2025

From what I can see the issue might be caused by the config returning a promise instead of an array

@ematipico
Copy link
Member

Are there really cases where a configuration might be a promise?

@Kamillaova
Copy link
Author

Are there really cases where a configuration might be a promise?

Probably, literally @antfu/eslint-config. And eslint works fine with this configuration.

@siketyan
Copy link
Member

siketyan commented Mar 3, 2025

it seems using top-level await for resolving the config antfu/eslint-config#323

@ematipico
Copy link
Member

That's very unfortunate, I'm not sure we can support this configuration with the current infrastructure. We rely on node and the result given by the executed eslint configuration file.

To support it, it means we need to ship a shim just to resolve the promise, very inconvenient. cc @Conaclos who worked on the feature

@Kamillaova
Copy link
Author

Can you provide the command with which biome tries to evaluate the config?

@siketyan
Copy link
Member

siketyan commented Mar 3, 2025

The config is loaded in

pub(crate) fn load_config(specifier: &str) -> Result<Resolution, CliDiagnostic> {
.

I noticed it's not a top-level await, just the default export is a Promise. It seems Biome already supports TLA, so export default await antfu(...) can be a workaround?

@siketyan
Copy link
Member

siketyan commented Mar 3, 2025

Maybe we can support such a config by flattening the Promise?
#5255

@Kamillaova
Copy link
Author

  ✖ Migration has encountered an error: `node` was invoked to resolve './eslint.config.js'. This invocation failed with the following error:
    [eval]:15
    } import('./eslint.config.js').then((c) => c.default).then((c) => console.log(JSON.stringify(uncycle(c))))
                                                                                       ^

    TypeError: Converting circular structure to JSON
        --> starting at object with constructor 'Object'
        |     property 'configs' -> object with constructor 'Object'
        |     property 'flat/base' -> object with constructor 'Array'
        |     ...
        |     property 'plugins' -> object with constructor 'Object'
        --- property 'jsonc' closes the circle
        at JSON.stringify (<anonymous>)
        at [eval]:15:84

    Node.js v22.8.0

:(

@pnodet
Copy link
Contributor

pnodet commented Mar 3, 2025

I think another part of the issue is that the antfu config doesn't return an array of eslint config objects in a typical way, it returns a Promise that resolves to a FlatConfigComposer

import { FlatConfigComposer } from 'eslint-flat-config-utils';

@Kamillaova
Copy link
Author

Idk, but I think this project might be useful for exploring how to most correctly convert a flat configuration into an object with settings of eslint, etc.

https://github.com/eslint/config-inspector

@pnodet
Copy link
Contributor

pnodet commented Mar 3, 2025

for people running into this issue, I used eslint print-config to extract the resulting configs and passed it to biome migrate.

@Kamillaova
Copy link
Author

@pnodet

Oops! Something went wrong! :(

ESLint: 9.17.0

No files matching the pattern "print-config" were found.
Please check for typing mistakes in the pattern.

@Conaclos
Copy link
Member

Conaclos commented Mar 3, 2025

for people running into this issue, I used eslint print-config to extract the resulting configs and passed it to biome migrate.

Note that eslint --print-config prints the resolved configuration for a given path. This can be ok if you apply the same config on all files. Usually this truncates the config. This is why we don't use it internally and invoke node ourselves.

Regarding the current issue, we could perhaps use await for awaiting any Promise or plain value.

@Kamillaova
Copy link
Author

eslint --print-config

Value for 'print-config' of type 'path::String' required.
You're using eslint.config.js, some command line flags are no longer available. Please see https://eslint.org/docs/latest/use/command-line-interface for details.

@siketyan
Copy link
Member

siketyan commented Mar 3, 2025

@Kamillaova Could you try eslint --print-config eslint.config.js?

@Kamillaova
Copy link
Author

@pnodet
Copy link
Contributor

pnodet commented Mar 3, 2025

@Conaclos yes, this is a dirty workaround. Regarding the current issue there are two underlying problems: the fact that it can return a promise and the fact that the promise here is not an array but instead a FlatConfigComposer

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

Successfully merging a pull request may close this issue.

5 participants