Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
chore: migrate fully to Yarn #3155
chore: migrate fully to Yarn #3155
Changes from 4 commits
df1f75d
3896961
d46274b
9781c8c
d2fe764
dc21e03
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we need this in every single repo that uses prettier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but it's only needed for Prettier and TS integration with VSCode and other IDEs.
Imo I think this is worth committing because it creates a working out of the box dev environment.
The code can be updated automatically vwith
yarn dlx @yarnpkg/sdks
, though the VS Code config file needs to be altered slightly in order to play nicely with our js folder being a subdirectory.https://yarnpkg.com/getting-started/editor-sdks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern is keeping this up to date across our (many) repos. What manual changes are needed, and could we automate them somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, it generates a vscode config file in the JS folder. We just move that out, and prepend
js/
to any paths needed.Looking now, I can't actually see the vscode config file in this PR... I need to add that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we could add something to flarum-cli. But I wonder if the huge pile of boilerplate we need to keep track of is worth it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's one of the things that won't be a 'huge pile' when we go monorepo eventually. Our root workspace could have Prettier installed with this boilerplate, and everything would work fine in subdirectories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not just us: monorepo or not, we can't sacrifice developer experience for third party devs.
I know these comments aren't particularly constructive. But I guess my point is that I'm not sure that Yarn 2 is worth it atm. It seems like most major players are still in the npm / yarn 1 system, and that npm itself is getting a lot better and more scalable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note that these changes you're referring to are specifically for Yarn PnP support rather than Yarn 2 or Yarn 3.
Reverting 9781c8c will let us use Yarn 3 without PnP using the legacy node_modules linker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, if we want to commit entirely to Yarn, I would prefer switching to Yarn 3 without PnP, and then reconsider PnP at a later time. Incremental steps!