🐛 eslint migration find/replace bug #3079

mjgerace · 2024-06-05T19:30:42Z

Environment information

CLI:
  Version:                      1.8.0
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           macos

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

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

Workspace:
  Open Documents:               0

What happened?

I make use of the eslint "extends" section, where I extend a few of my own rules like so:

  extends: [
    "@mjgerace/eslint-configs/base-vitest",
    "@mjgerace/eslint-configs/react",
    "@mjgerace/eslint-configs/graphql",
  ],

When running npx @biomejs/biome migrate eslint --write, I am hitting the following issue, repeated for every one of the extends imports above:

✖ Migration has encountered an error: `node` was invoked to resolve '@mjgerace/eslint-config-eslint-configs/graphql'. This invocation failed with the following error:
    node:internal/modules/esm/resolve:854
      throw new ERR_MODULE_NOT_FOUND(packageName, fileURLToPath(base), null);
            ^
    
    Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@mjgerace/eslint-config-eslint-configs' imported from /Users/mjgerace/repos/MjApp/app/[eval]

Something fascinating is happening here - somewhere in the migration script, it appears biome is seeing @mjgerace/eslint-configs, and transforming it to @mjgerace/eslint-config-eslint-configs. How bizarre!

Expected result

The imports are left unchanged so that node can resolve them, as eslint does today.

Code of Conduct

I agree to follow Biome's Code of Conduct

The text was updated successfully, but these errors were encountered:

Conaclos · 2024-06-05T20:56:39Z

Something fascinating is happening here - somewhere in the migration script, it appears biome is seeing @mjgerace/eslint-configs, and transforming it to @mjgerace/eslint-config-eslint-configs. How bizarre!

ESLint has some magic under the hood. biome migrate eslint tries to mimic that.
I followed the ESLInt docs: for scoped package, Biome checks that the scoped part is either eslint-config or starts with eslint-config-; otherwise it prepends eslint-config-.
Maybe I missed something. Have you some context I don't have?

EDIT: maybe the ESLInt docs is erroneous and ESLint checks eslint-config prefix instead of eslint-config- prefix.

Conaclos · 2024-06-06T10:04:01Z

I tried to test your config with ESLint and it seems that it also fails.
Here my reproduction:

❯ npm init
❯ npm i -D --save-exact [email protected]
❯ mkdir -p node_modules/@mjgerace/eslint-configs
❯ cat node_modules/@mjgerace/eslint-configs/package.json
  {
    "name": "eslint-configs",
    "version": "1.0.0",
    "main": "index.js",
    "exports": {
      "./base-vitest": "./index.js"
    }
  }

❯ cat node_modules/@mjgerace/eslint-configs/index.js 
  module.exports = {}

❯ cat .eslintrc.json 
  {
    "extends": ["@mjgerace/eslint-configs/base-vitest"]
  }

❯ npx eslint -c .eslintrc.json 

  Oops! Something went wrong! :(

  ESLint: 8.56.0

  ESLint couldn't find the config "@mjgerace/eslint-configs/base-vitest" to extend from. Please check that the name of the config is correct.

  The config "@mjgerace/eslint-configs/base-vitest" was referenced from the config file in ".eslintrc.json".

  If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.

❯ # Even the base config failed to be loaded:
❯ cat .eslintrc.json 
  {
    "extends": ["@mjgerace/eslint-configs"]
  }

❯ npx eslint -c .eslintrc.json 

  Oops! Something went wrong! :(

  ESLint: 8.56.0

  ESLint couldn't find the config "@mjgerace/eslint-configs" to extend from. Please check that the name of the config is correct.

  The config "@mjgerace/eslint-configs" was referenced from the config file in ".eslintrc.json".

  If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.

mjgerace · 2024-06-06T18:03:49Z

I tried to test your config with ESLint and it seems that it also fails. Here my reproduction:

Let me get you more info. I can assure you this setup works and has worked for a while - perhaps not having that package defined in package.json (despite having it in node_modules) is causing this issue. Something about this works fine for me for vanilla eslint, again, this idea is completely unrelated from the weird path modification I see happening -

that is to say, I think that this works great in general, and even if it didn't, the path modification for biome migrate doesn't match eslint quite as it should. You can even test biome migrate to see what I am talking about (run it on your example!)

Conaclos · 2024-06-06T20:59:53Z

You can even test biome migrate to see what I am talking about (run it on your example!)

I know it will fail, and I will get the same error you got.

However, to be able to fix the issue, I must be sure that your example is valid in ESLint. My reproduction of your example shows that it is not valid in ESLint. Could you provide a reproduction that is valid for ESLint and invalid for Biome?

Conaclos · 2024-06-24T13:21:11Z

I am not sure if it is a real issue. It seems that this also fails with ESLint. Closing because it is not actionable.

mjgerace · 2024-06-25T18:34:55Z

@Conaclos I will follow up, this is a real issue. I will provide evidence to reopen.

TheSamsa · 2024-10-28T08:30:33Z

Hi there, I have the same Issue.

CLI:
  Version:                      1.9.4
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           linux

Environment:
  BIOME_LOG_PATH:               unset
  BIOME_LOG_PREFIX_NAME:        unset
  BIOME_CONFIG_PATH:            unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v22.9.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "yarn/1.22.19"

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

Workspace:
  Open Documents:               0

package.json devDependency: @samsa/eslint-config
.eslintrc.js useage extends: [ @samsa/eslint-config/nestjs ]`

The command yarn biome migrate eslint --write throws:

Migration has encountered an error: `node` was invoked to resolve '@samsa/eslint-config-eslint-config/nestjs'. This invocation failed with the following error:
    node:internal/modules/esm/resolve:839
      throw new ERR_MODULE_NOT_FOUND(packageName, fileURLToPath(base), null);

as you can see it prepends eslint-config- to the name of the module lookup.

I also looked at the biome code, and in file crates/biome-cli/src/execute/migrate/eslint.rs if I replace the line with:
Cow::Owned(format!("{scope}/{scoped}")) it returns the expected @samsa/eslint-config/nestjs String/Cow.
Maybe there is an additional check needed?

Oh btw, I can ensure this setup works in ESLint, It's beeing used this way for a while now.

Conaclos · 2024-10-28T09:24:28Z

@TheSamsa Actually I already fixed it in #2705
Which version of Biome are you using?

ematipico · 2024-10-28T09:58:45Z

@TheSamsa Actually I already fixed it in #2705
Which version of Biome are you using?

1.9.4 based on the rage command

Conaclos · 2024-10-28T10:06:27Z

@TheSamsa Could you provide a reproduction? @samsa/eslint-config is not published on npm...

TheSamsa · 2024-10-28T10:09:57Z

Its version 1.9.4, the most recent one in yarnpkgs.
@Conaclos I will tomorrow if thats okay. I used my name as placeholder since I have to check if its okay with the company to share this.
I will have to reproduce it on GH then, since its a private repository.

TheSamsa · 2024-10-29T12:07:35Z

@Conaclos I've provided a public repo with the basics of the eslint-config stuff.
https://github.com/TheSamsa/eslint_config

Conaclos · 2024-10-30T14:04:16Z

@TheSamsa Thanks for the repro. Fixed in #4435

TheSamsa · 2024-10-30T14:17:47Z

Brilliant, thanks a lot!
I am really curious to test biome!

Conaclos · 2024-10-30T15:58:54Z

@TheSamsa I released a nightly version if you want to try it.

Conaclos added the S-Bug-confirmed Status: report has been confirmed as a valid bug label Jun 5, 2024

Conaclos self-assigned this Jun 5, 2024

Conaclos added S-Needs repro Status: needs a reproduction S-Needs response Status: await response from OP and removed S-Bug-confirmed Status: report has been confirmed as a valid bug labels Jun 6, 2024

Conaclos closed this as not planned Won't fix, can't repro, duplicate, stale Jun 24, 2024

Conaclos mentioned this issue Oct 30, 2024

fix(migrate-eslint): corretcly handle scoped package with paths #4435

Merged

Conaclos closed this as completed in #4435 Oct 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 eslint migration find/replace bug #3079

🐛 eslint migration find/replace bug #3079

mjgerace commented Jun 5, 2024 •

edited

Loading

Conaclos commented Jun 5, 2024 •

edited

Loading

Conaclos commented Jun 6, 2024 •

edited

Loading

mjgerace commented Jun 6, 2024 •

edited

Loading

Conaclos commented Jun 6, 2024

Conaclos commented Jun 24, 2024

mjgerace commented Jun 25, 2024 •

edited

Loading

TheSamsa commented Oct 28, 2024 •

edited

Loading

Conaclos commented Oct 28, 2024 •

edited

Loading

ematipico commented Oct 28, 2024

Conaclos commented Oct 28, 2024

TheSamsa commented Oct 28, 2024 •

edited

Loading

TheSamsa commented Oct 29, 2024 •

edited

Loading

Conaclos commented Oct 30, 2024 •

edited

Loading

TheSamsa commented Oct 30, 2024 •

edited

Loading

Conaclos commented Oct 30, 2024

🐛 eslint migration find/replace bug #3079

🐛 eslint migration find/replace bug #3079

Comments

mjgerace commented Jun 5, 2024 • edited Loading

Environment information

What happened?

Expected result

Code of Conduct

Conaclos commented Jun 5, 2024 • edited Loading

Conaclos commented Jun 6, 2024 • edited Loading

mjgerace commented Jun 6, 2024 • edited Loading

Conaclos commented Jun 6, 2024

Conaclos commented Jun 24, 2024

mjgerace commented Jun 25, 2024 • edited Loading

TheSamsa commented Oct 28, 2024 • edited Loading

Conaclos commented Oct 28, 2024 • edited Loading

ematipico commented Oct 28, 2024

Conaclos commented Oct 28, 2024

TheSamsa commented Oct 28, 2024 • edited Loading

TheSamsa commented Oct 29, 2024 • edited Loading

Conaclos commented Oct 30, 2024 • edited Loading

TheSamsa commented Oct 30, 2024 • edited Loading

Conaclos commented Oct 30, 2024

mjgerace commented Jun 5, 2024 •

edited

Loading

Conaclos commented Jun 5, 2024 •

edited

Loading

Conaclos commented Jun 6, 2024 •

edited

Loading

mjgerace commented Jun 6, 2024 •

edited

Loading

mjgerace commented Jun 25, 2024 •

edited

Loading

TheSamsa commented Oct 28, 2024 •

edited

Loading

Conaclos commented Oct 28, 2024 •

edited

Loading

TheSamsa commented Oct 28, 2024 •

edited

Loading

TheSamsa commented Oct 29, 2024 •

edited

Loading

Conaclos commented Oct 30, 2024 •

edited

Loading

TheSamsa commented Oct 30, 2024 •

edited

Loading