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

fix: the .prettierignore file did not take effect during precommit #2744

Merged

Conversation

WhiteMinds
Copy link
Contributor

When Lerna executes package scripts, it uses the package directory as the execution directory.
However, Prettier only reads the .prettierignore file in the execution directory, so the root-level .prettierignore file is not effective when running npx lerna run --no-bail --stream precommit.

Since VSCode only reads the .prettierignore file in the opened folder, the file must exist in the root directory. Therefore, the current solution is adapted from prettier/prettier#4081 (comment).

@Keith-CY
Copy link
Collaborator

When Lerna executes package scripts, it uses the package directory as the execution directory. However, Prettier only reads the .prettierignore file in the execution directory, so the root-level .prettierignore file is not effective when running npx lerna run --no-bail --stream precommit.

Since VSCode only reads the .prettierignore file in the opened folder, the file must exist in the root directory. Therefore, the current solution is adapted from prettier/prettier#4081 (comment).

Is it feasible to run prettier in the root directory instead of run prettier by lerna in the package directories

@github-actions
Copy link
Contributor

Packaging for test is done in 5386706617 for commit 7f872e1 .

@WhiteMinds
Copy link
Contributor Author

WhiteMinds commented Jun 27, 2023

Is it feasible to run prettier in the root directory instead of run prettier by lerna in the package directories

It can be a bit complex, because the current process is:

git commit -> .git/hooks/pre-commit -> .husky/pre-commit -> npx lerna run --no-bail --stream precommit ->
[multiple package parallel] package:precommit -> package:lint-staged -> prettier & eslint.

To achieve the goal, you need to extract prettier from lint-staged and move it to .husky/pre-commit.
This will make the management of prettier isolated.

@Keith-CY
Copy link
Collaborator

Keith-CY commented Jun 27, 2023

Is it feasible to run prettier in the root directory instead of run prettier by lerna in the package directories

It can be a bit complex, because the current process is:

git commit -> .git/hooks/pre-commit -> .husky/pre-commit -> npx lerna run --no-bail --stream precommit ->
[multiple package parallel] package:precommit -> package:lint-staged -> prettier & eslint.

To achieve the goal, you need to extract prettier from lint-staged and move it to .husky/pre-commit. This will make the management of prettier isolated.

I can understand all except

This will make the management of prettier isolated.

What does isolated mean if prettier is executed in the root directory

@WhiteMinds
Copy link
Contributor Author

What does isolated mean if prettier is executed in the root directory

What I mean is that it will no longer be a part of lint-staged that is based on each package's independent configuration, but rather something configured relative to the root level.

I hope that this kind of individual devops for each package should be maintained as much as possible within the package itself.

@Keith-CY
Copy link
Collaborator

What does isolated mean if prettier is executed in the root directory

What I mean is that it will no longer be a part of lint-staged that is based on each package's independent configuration, but rather something configured relative to the root level.

I hope that this kind of individual devops for each package should be maintained as much as possible within the package itself.

Reasonable and let's keep prettier independent in packages

Copy link
Collaborator

@homura homura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Keith-CY Keith-CY added this pull request to the merge queue Jun 27, 2023
Merged via the queue into nervosnetwork:develop with commit 815a965 Jun 27, 2023
10 checks passed
@WhiteMinds WhiteMinds deleted the fix/PrettierIgnoreFileInvalid branch June 27, 2023 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants