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: plugins parser not inferred #3027

Merged
merged 14 commits into from Jun 27, 2023
Merged

Conversation

u3u
Copy link
Contributor

@u3u u3u commented Jun 14, 2023

Fixed #3026

  • Add test
  • Upgrade PNPM version to v8

After running yarn test without changing anything locally, I had 2 test failures. But I don't know why. Is it a problem with my local environment?

  • Run tests
  • Update the CHANGELOG.md with a summary of your changes

@auto-assign auto-assign bot requested a review from ntotten June 14, 2023 15:41
@u3u
Copy link
Contributor Author

u3u commented Jun 16, 2023

image

After confirmation, it was found that the reason for the failure of the following two tests was due to the existence of ~/.prettierrc.cjs file in my local environment. After temporarily renaming it, all tests have passed.

  • it uses config from .editorconfig file
  • it uses config from vscode settings

In addition, because I use 8.6.2 for my local PNPM version, the version of lock file test-fixtures/plugins-pnpm/pnpm-lock.yaml will be updated. However, PNPM 8 requires Node.js 16 or above, while GitHub Actions currently uses Node.js 14, which will cause the operation to fail.

https://github.com/u3u/prettier-vscode/actions/runs/5269104746/jobs/9526747643

- uses: pnpm/action-setup@v2.2.4
with:
version: 7
run_install: false
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: "14"

@u3u u3u force-pushed the fix/plugins-inferred-parser branch from 1618987 to 5dce927 Compare June 16, 2023 15:02
@ntotten
Copy link
Member

ntotten commented Jun 18, 2023

Looks pretty good, there is one test failing though. Can you take a look?

@u3u
Copy link
Contributor Author

u3u commented Jun 19, 2023

@ntotten Hi, this is a bit strange. All tests passed before you merged the main branch, and after I updated it, I ran the tests locally again and they also passed.

All passed: https://github.com/prettier/prettier-vscode/actions/runs/5302709062/jobs/9600483439
Partial passed: https://github.com/prettier/prettier-vscode/actions/runs/5304290262/jobs/9601182513

The failed test is "it formats by Prettier v3", which should not be related to my changes.

And now there are strange errors in the main branch testing, which makes me have to suspect whether it is caused by this commit 🤔 (822ad24)
https://github.com/prettier/prettier-vscode/actions/runs/5304286381/jobs/9600500159

@u3u

This comment was marked as resolved.

@u3u
Copy link
Contributor Author

u3u commented Jun 21, 2023

I don't understand why using pnpm 7 would affect this test.
There is no problem testing pnpm 8 on non-Windows systems now.

image

run `corepack prepare pnpm@7.33.1 --activate`
@u3u
Copy link
Contributor Author

u3u commented Jun 21, 2023

https://github.com/u3u/prettier-vscode/actions/runs/5330997873
All tests have passed now. Since Windows cannot use Node.js 18 for testing in CI, only pnpm 7 can be used.

To pass the "it formats by Prettier v3" test when using pnpm 7, lockfileVersion of pnpm-lock.yaml needs to be restored to 5.4. I don't know why it is necessary. For future compatibility, I kept the file .npmrc under test-fixtures/plugins-pnpm/.

@ntotten ntotten merged commit 1afa77f into prettier:main Jun 27, 2023
4 checks passed
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.

prettier.getFileInfo should pass plugins to help infer the plugin's inferredParser.
3 participants