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

Switch to Yarn plug-and-play #738

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Switch to Yarn plug-and-play #738

wants to merge 1 commit into from

Conversation

Borewit
Copy link
Owner

@Borewit Borewit commented Aug 5, 2024

@Borewit Borewit added the DevOps DevOps / Continuous integration label Aug 5, 2024
@Borewit Borewit self-assigned this Aug 5, 2024
@Borewit Borewit force-pushed the switch-to-yarn-pnp branch from e74dff8 to f3fff70 Compare August 5, 2024 15:32
@Borewit
Copy link
Owner Author

Borewit commented Aug 5, 2024

@minht11, looks like both @biomejs/biome and remark-cli are not pnp compliant.

@minht11
Copy link
Contributor

minht11 commented Aug 5, 2024

@Borewit I have zero experience with yarn pnp, but by the looks of it it tries to lint those files. You can just ignore them in biome.jsonc files.ignore

"./.pnp.cjs",
"./.pnp.loader.mjs"

Tested locally.

@Borewit Borewit force-pushed the switch-to-yarn-pnp branch from f3fff70 to 72adacf Compare August 5, 2024 19:55
package.json Outdated
@@ -42,7 +42,7 @@
"lib/**/*.d.ts"
],
"devDependencies": {
"@biomejs/biome": "1.8.3",
"@biomejs/biome": "^1.8.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Biome minor version can have new rules, additional fixes enabled between versions, they specifically recommend pinning its version to prevent unexpected breakage
image

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think the version is already pinned using yarn, in yarn.lock.

Any update typically needs to pass CI, usually triggered via dependabot.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you have already installed dependencies then yes, but if for example you clone new project ant run npm install it will attempt to download never dependencies matching range, not sure on how specifically yarn handles it, but in this case I think it's better to pin it.

@Borewit Borewit force-pushed the switch-to-yarn-pnp branch from 72adacf to 7326bf7 Compare August 5, 2024 20:04
@Borewit
Copy link
Owner Author

Borewit commented Aug 5, 2024

@Borewit I have zero experience with yarn pnp, but by the looks of it it tries to lint those files. You can just ignore them in biome.jsonc files.ignore

"./.pnp.cjs",
"./.pnp.loader.mjs"

Tested locally.

It is as if the configuration file is ignored when switching to pnp. You need to run yarn install to test this branch (peek-readable/node_modules should disappear).

@minht11
Copy link
Contributor

minht11 commented Aug 5, 2024

The latest breakage is because you enabled formatter.enabled to true, you can either disable it or run yarn lint --fix

@Borewit Borewit force-pushed the switch-to-yarn-pnp branch from 7326bf7 to e82f75b Compare August 5, 2024 20:20
@Borewit
Copy link
Owner Author

Borewit commented Aug 5, 2024

Great, only remark-cli remaining

@Borewit Borewit force-pushed the switch-to-yarn-pnp branch from e82f75b to 9d2ab51 Compare August 5, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DevOps DevOps / Continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants