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

🐛 Migration quit on node warning. #3104

Closed
1 task done
fudom opened this issue Jun 7, 2024 · 8 comments · Fixed by #3259 · 4 remaining pull requests
Closed
1 task done

🐛 Migration quit on node warning. #3104

fudom opened this issue Jun 7, 2024 · 8 comments · Fixed by #3259 · 4 remaining pull requests
Labels
good first issue Good for newcomers S-Enhancement Status: Improve an existing feature S-Help-wanted Status: you're familiar with the code base and want to help the project

Comments

@fudom
Copy link

fudom commented Jun 7, 2024

Environment information

CLI:
  Version:                      1.8.0
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           windows

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         unset
  JS_RUNTIME_VERSION:           "v20.14.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "npm/10.7.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?

npx @biomejs/biome migrate eslint --write
migrate ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  × Migration has encountered an error: `node` was invoked to resolve './eslint.config.mjs'. This invocation failed with the following error:
    (node:18172) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
    (Use `node --trace-warnings ...` to show where the warning was created)

Expected result

Node just prints a warning. No reason to quit.

Code of Conduct

  • I agree to follow Biome's Code of Conduct

Additional information

Btw. biome migrate is deprecated I guess. Because they do nothing. See the docs (top).
The new command is @biomejs/biome migrate which is a few sections below.
https://biomejs.dev/guides/migrate-eslint-prettier/

@fudom fudom changed the title 🐛 <TITLE> 🐛 Migration quit on node warning. Jun 7, 2024
@Sec-ant
Copy link
Member

Sec-ant commented Jun 7, 2024

Btw. biome migrate is deprecated I guess. Because they do nothing. See the docs (top).\nThe new command is @biomejs/biome migrate which is a few sections below.\nhttps://biomejs.dev/guides/migrate-eslint-prettier/

@biomejs/biome is the node package name and biome is the binary name. You should use biome if you install it via homebrew, or if you use npm and install @biomejs/biome globally.

@fudom
Copy link
Author

fudom commented Jun 7, 2024

Thanks my fault. I used the old package biome. But not now. I installed the correct package. The npx command is correct. I also installed it now locally and can use the cmd biome within the node scripts if package.json. It works. The problem is, that the migration scripts aborts because node prints a warning. I would expect only abort on errors.

@Sec-ant
Copy link
Member

Sec-ant commented Jun 7, 2024

Sorry for the confusion, I just reply to say that we didn't deprecated it. Maybe the docs can be improved.

The problem is, that the migration scripts aborts because node prints a warning. I would expect only abort on errors.

Yeah this should be a bug.

@fudom
Copy link
Author

fudom commented Jun 7, 2024

That's what I thought after I sent my post. Sorry ^^ I was confused because I used the command npx biome (wrong package) instead of npx @biomejs/biome. But anyway... It's all good now.

To the problem: I used import foo from './bar.json' assert { type: 'json' }; which is experiemental in node (v20.14) and prints a warning. But I replaced it now with fs readFileSync for the migration. Now it worked. Not sure if biome aborts on any print output or only on warnings. But yes, it should only stop on error.

@ematipico
Copy link
Member

I believe the warning was printed by Node.js on stderr, and that's where errors are usually printed too. The migration assumes that, so the warning is misinterpreted as an error.

That's surely a case we didn't take into consideration.

@fudom
Copy link
Author

fudom commented Jun 7, 2024

Maybe. I don't know. ESLint had no problem with it. I didn't tested other warnings. I'm fine with closing this until someone else has the problem...

@Conaclos
Copy link
Member

Conaclos commented Jun 7, 2024

We could try to set NODE_NO_WARNINGS=1 when running node. This should suppress any warning.
By the way could you try to run NODE_NO_WARNINGS=1 npx @biomejs/biome migrate eslint?

@ematipico
Copy link
Member

ematipico commented Jun 11, 2024

@fudom would you like to a PR? It should be very easy to add NODE_NO_WARNINGS=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers S-Enhancement Status: Improve an existing feature S-Help-wanted Status: you're familiar with the code base and want to help the project
Projects
None yet
4 participants