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

Enable corepack & fix pre-commit run --all-files #462

Closed
wants to merge 5 commits into from

Conversation

Mikaela
Copy link
Member

@Mikaela Mikaela commented Jun 25, 2024

Briefly talked on Matrix, I think the project was previously using an old yarn version since there was a warning about the project being updated to new yarn version when I ran corepack use yarn.

I don't know if this brings breaking changes and I am curious on whether the tests or CI pass.

@@ -0,0 +1 @@
nodeLinker: node-modules
Copy link
Member Author

Choose a reason for hiding this comment

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

Specifying pnpm here might conserve disk space

mx-tester.yml Outdated Show resolved Hide resolved
…pack more

At least yarn build didn't break locally for me, so I think it's supported here too
@Mikaela
Copy link
Member Author

Mikaela commented Jun 26, 2024

Additionally it's not necessary to call corepack constantly since corepack enable should make yarn point to it, but I am choosing the lazy way out of having to figure out how to get the CI to use corepack version instead of Yarn Classic which is installed by default.

It's inspired by https://github.com/nodejs/corepack/blob/v0.28.2/README.md#corepack-enable--name

@Mikaela
Copy link
Member Author

Mikaela commented Jun 26, 2024

Scimming through the CI logs it seems that it's unable to find Draupnir version, which I think is related to this

I think that needs actual coder to check it thogh.

@Gnuxie
Copy link
Member

Gnuxie commented Jun 26, 2024

Yeah that's a known issue and unrelated to this PR, don't worry about it

@Gnuxie
Copy link
Member

Gnuxie commented Jun 29, 2024

@Mikaela did you follow https://yarnpkg.com/migration/guide ?

@@ -77,5 +77,6 @@
},
"engines": {
"node": ">=18.0.0"
}
},
"packageManager": "yarn@4.3.1+sha512.af78262d7d125afbfeed740602ace8c5e4405cd7f4735c08feb327286b2fdb2390fbca01589bfd1f50b1240548b74806767f5a063c94b67e431aabd0d86f7774"
Copy link
Member

Choose a reason for hiding this comment

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

Why is there a sha512?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is what corepack use yarn gave me

@Mikaela
Copy link
Member Author

Mikaela commented Jun 29, 2024

More or less, although by accident

@Gnuxie
Copy link
Member

Gnuxie commented Jun 29, 2024

oki

@Gnuxie
Copy link
Member

Gnuxie commented Jun 29, 2024

Been playing around with yarn4 just to make sure everything is in order, unfortunately I haven't been able to get yarn link working with VSCode's debugger when linking MPS locally. And that seems to be because they no longer use a symlink in node_modules.

https://classic.yarnpkg.com/lang/en/docs/cli/link/
https://yarnpkg.com/cli/link
https://yarnpkg.com/configuration/manifest#resolutions
https://yarnpkg.com/protocol/portal

I'm investigating this but it might take awhile.

@Mikaela
Copy link
Member Author

Mikaela commented Jun 29, 2024

@Gnuxie
Copy link
Member

Gnuxie commented Jun 30, 2024

Ok, that looks promising. However, seems like projects that have been touched by yarn classic aren't automatically migrated to use "yarn plug'n'play" which is required for the editor integration. https://yarnpkg.com/migration/pnp.
And that migration guide requires some major work.

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.

2 participants