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

resolveConfig(cwd) fails to find config file #15879

Closed
mbeckem opened this issue Jan 5, 2024 · 10 comments · Fixed by #15910
Closed

resolveConfig(cwd) fails to find config file #15879

mbeckem opened this issue Jan 5, 2024 · 10 comments · Fixed by #15910
Labels
help wanted We're a small group who can't get to every issue promptly. We’d appreciate help fixing this issue! locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:docs Issues about adding or improving documentation

Comments

@mbeckem
Copy link

mbeckem commented Jan 5, 2024

Environments:

  • Prettier Version: 3.1.1
  • Running Prettier via: Node
  • Runtime: Node 18
  • Operating System: Linux
  • Prettier plugins (if any): None

Steps to reproduce:

  1. Open https://stackblitz.com/edit/node-37bnjb?file=test.js
  2. Run node ./test.js

Expected behavior:

The script prints the parsed contents of the .prettierrc inside that directory, since cwd is an explicitly supported input parameter (see https://prettier.io/docs/en/api.html#prettierresolveconfigfileurlorpath--options)

Actual behavior:

The script prints null.

Notes:

  • Downgrade prettier to version 2 by editing the package.json followed by running npm install.
    • node ./test.js will print the expected output
  • The resolveConfig function in prettier v3 appears to start its search in dirname(cwd) instead of cwd, thus never testing for the existence of .prettierrc in the project directory.
@fisker
Copy link
Member

fisker commented Jan 5, 2024

Broken by #15363, didn't know it's documented, I thought it's bug..

@fisker fisker added the status:needs discussion Issues needing discussion and a decision to be made before action can be taken label Jan 5, 2024
@fisker
Copy link
Member

fisker commented Jan 5, 2024

Maybe we can restore the old behavior, and add a new .resolveConfigForFile()?

@fisker
Copy link
Member

fisker commented Jan 5, 2024

It's really bad that it accepts a dir path, which make no sense at all. Config files can have overrides, without file path the resolved config can be totally wrong.

@mbeckem
Copy link
Author

mbeckem commented Jan 5, 2024

I agree that always passing the path to the actual file would be the best approach moving forward. However, this is a pretty hard break in documented behavior. Is it possible restore (but deprecate) the old behavior for compatibilty and include a warning in the docs, so the old behavior can be removed at a later time? There might be other tools that rely on the old directory lookup.

@fisker fisker added this to the 3.2 milestone Jan 7, 2024
@fisker
Copy link
Member

fisker commented Jan 7, 2024

Is it possible restore (but deprecate) the old behavior for compatibilty and include a warning in the docs, so the old behavior can be removed at a later time?

I agree this is reasonable, but we'll have to add another usable/correct api, and it will be hard (and take a long time) to remove the new added one again. I highly doubt there are more broken cases.

I say, we should just update the docs to reflect the current behavior.

@sosukesuzuki need your opinion.

@sosukesuzuki
Copy link
Member

@fisker I agree with you. It is enough to update the docs I think. If the same problem is reported repeatedly in the future, we can consider again.

@fisker
Copy link
Member

fisker commented Jan 10, 2024

Would you like to update it?

@sosukesuzuki
Copy link
Member

If you can do it, I want you to do it.

@fisker
Copy link
Member

fisker commented Jan 10, 2024

Going to sleep..maybe later.

@fisker fisker added help wanted We're a small group who can't get to every issue promptly. We’d appreciate help fixing this issue! type:docs Issues about adding or improving documentation and removed status:needs discussion Issues needing discussion and a decision to be made before action can be taken labels Jan 10, 2024
@sosukesuzuki
Copy link
Member

@fisker If we should just update the documentation, can we remove this from 3.2 milestone?

@fisker fisker removed this from the 3.2 milestone Jan 11, 2024
elieux added a commit to elieux-contrib/prettier-vscode that referenced this issue Jan 17, 2024
... not for the workspace directory.

Passing a directory to resolveConfig was broken in prettier v3.1.1, as reported here:
prettier/prettier#15879
elieux added a commit to elieux-contrib/prettier-vscode that referenced this issue Jan 17, 2024
... not for the workspace directory.

Passing a directory to resolveConfig was broken in prettier v3.1.1, as reported here:
prettier/prettier#15879
elieux added a commit to elieux-contrib/prettier-vscode that referenced this issue Jan 18, 2024
... not for the workspace directory.

Passing a directory to resolveConfig was broken in prettier v3.1.1, as reported here:
prettier/prettier#15879
@github-actions github-actions bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Apr 14, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted We're a small group who can't get to every issue promptly. We’d appreciate help fixing this issue! locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:docs Issues about adding or improving documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants