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

Feature/yarn3 - migrate to the new version of yarn #326

Merged
merged 23 commits into from
Nov 17, 2021

Conversation

ja0nz
Copy link
Contributor

@ja0nz ja0nz commented Nov 12, 2021

oh boy...
I got carried away while playing around:)
This PR introduces Yarn3 to @thi.ng. Lerna is obsolete in the new setting and therefore gone.
Every commit is well documented (I hope). I am happy to answer every question.

This commit just covers the below init ceremony and this commit
can be dropped for security reasons.
Run this commands (in the repo):

yarn set version berry
yarn plugin import typescript
yarn plugin import workspace-tools
yarn plugin import interactive-tools
yarn install
- added examples/ to workspaces
yarn3 does not accept the previous nested approach, the examples
won't link proper and will yell with error messages

- removed lerna dep

- new build scripts based on yarn workspaces
ATTENTION: I didn't test all! Yarn npm publish needs probably
configuration:
https://yarnpkg.com/configuration/yarnrc
- pulled in some devDependencies
yarn3 complains about missing binaries (rimraf, tsc) therefore pulled in
all defDeps. also added api-extractor which was hardlinked previously
see prev commit for more information what changed
- expansion of devDependencies
Previously linked via hardcoded path. While this approach still
works in yarn3 I changed it in order to ensure portability. As
they are hardlinked to the workspace root (examples are now part
of a shared workspace) this won't blow up in size.

- added "workspace:^" link
use workspace version of @thi.ng dependencies
see prev commit for more information what changed
Examples are now part of the workspace. Therefore it might
be a good idea to set them private to avoid accidental
publishing. This should not happen anyway but I thought
its still a good idea to be precise about it.
Fixes warning:
<pkgs> doesn't provide @types/node, requested by ts-node
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 6046 lines exceeds the maximum allowed for the inline comments feature.

@earthlyreason
Copy link
Contributor

Thanks @ja0nz, this is quite a lift!

I'm just an observer, but I had a couple of questions (as someone else who would like less Lerna in his life).

Why yarn and not npm workspaces? I think yarn was used here already, but I'm curious if there are other considerations.

Just glancing, I noticed that @types/node was being added to dependencies, when I believe it could be added to devDependencies, since it is not needed at runtime.

Again, I'm just a bystander & mainly asking out of curiosity. Thanks again for your work on this!

@ja0nz
Copy link
Contributor Author

ja0nz commented Nov 12, 2021

Hey @gavinpc-mindgrub

thanks for the quick feedback! Speaking of Lerna this was actually the starting point of this unplanned port. Lerna is at around 660 open issues and shows lack of maintenance lerna/lerna#2703. While this project uses yarn1 indeed, the workspaces features matured a lot in yarn2 and yarn3! For example, it is impossible to build a workspace solely with yarn1 - yarn3 supports topologic build order and parallel build process. There are far more improvements of course
See https://dev.to/arcanis/yarn-3-0-performances-esbuild-better-patches-e07

I don't know much about npm workspaces but I suppose they are similar to yarn:)

Thank you for pointing me towards @types/node. I just took the installation instruction from https://www.npmjs.com/package/@types/node and didn't thought about it a lot. I may shift them to devDependencies after the first review.

@postspectacular
Copy link
Member

OH BOY, INDEED @ja0nz! 😅

Firstly, welcome & thank you so much for this mega lift! I've just checked it out locally and it really all seems to work - BRAVO! This switch is something I've been scared of and deferring so many times myself, so I'm super glad you picked this up and showed the way...

Since I don't know yet much myself about yarn3, can you please summarize the overall new behavior/repo setup? I've seen your updated script aliases in the main pkg.json and it's working beautifully. I was under the impression though, that the default now is that new PnP resolution logic, yet I'm still also seeing a node_modules folder (which I thought would not be used anymore). I guess, I'll have a lot of reading to do over the coming days... :) Any further pointers are highly appreciated!

Re: GH actions - the current test workflow config seems to be incompatible with the new repo. Maybe you could overwrite that file with this updated version (plus maybe any other changes I still don't know of?):

name: test-all
on:
    push:
        branches:
            - feature/*
            - develop
            - main
        paths:
            - "packages/**"
            - ".github/workflows/**"
            - "package.json"
            - "!**.md"
    pull_request:
        branches:
            - develop
jobs:
    test:
        runs-on: ubuntu-latest
        steps:
            - uses: actions/checkout@v2
            - uses: actions/setup-node@v2
              with:
                  node-version: ">=16.9.0"
                  cache: "yarn"
            - run: yarn test

Other questions: Where are those .cjs files (plugins & releases) in the new .yarn come from? Are they configured artefacts? Where are they listed?

Thanks again so much! +1 for leaving lerna behind - well aware it's been dead in the water for a while now...

@postspectacular
Copy link
Member

So after reading more about the yarn3 release workflow, I'm not sure anymore if this PR is sufficient enough to fully replace lerna. For day-to-day stuff yes, very likely. However, my number one important feature of lerna is that it automatically figures out which packages have been updated and in which way (major/minor/patch) AND that it auto-generates changelogs from the conventional commit messages. For this repo, I cannot live without these features and any (even minor) manual approaches like having to explicitly call yarn version minor --deferred is not gonna work for me... I tend to work on several packages at the same time and not having to spend cognitive bandwidth on keeping track which parts I changed (or sometimes defer for later) in which capacity is a huge help... Do you know of anything comparable for yarn3?

@ja0nz
Copy link
Contributor Author

ja0nz commented Nov 13, 2021

Hello @postspectacular and thank you for the warm welcome! Glad this PR is received so well. I will go over the mentioned topics one by one.

  • Re: nodeLinker: pnp vs node-modules (vs pnpm)
    I tried the new PnP linking initially on this project and was greeted with errors:) Its a nice idea to follow up but for the moment it generates too much friction and needs certainly more feedback loops. One big issue is certainly tooling/IDE which resolves a lot around node_modules. See https://yarnpkg.com/getting-started/editor-sdks
    I played around with the other two available linkers - pnpm and node-modules - and ended up committed settings. Some stats if you don't mind (from a smaller subset of @thi.ng):
nodeLinker nmMode time - yarn build size - node_modules
node-modules hardlinks-global 1:04 min 264 M
pnpm hardlinks-global / -local 1:02 min 286 M
node-modules hardlinks-local 1:04 min 287 M
pnpm classic 1:02 min 294 M
node-modules classic 1:04 min 304 M
  • Re: error in GH actions
    I am pretty sure it's just because I excluded the yarn.lock from this PR (to reduce noise). I will include it again in one of the following commits.

  • Re: .cjs files
    Yep, those are artifacts hosted here https://github.com/yarnpkg/berry/tree/master/packages.
    You probably already got across the official documentation https://yarnpkg.com/features/plugins

  • Re: yarn release workflow
    Aw, I wasn't aware of this workflow feature. Unfortunately I am not aware of an adequate replacement just yet. Can you probably elaborate a bit more on this? Is this a feature done automatically by lerna when running lerna publish or is there some custom scripting involved? I will follow up on this topic.

I may pull in lerna back again temporary for this use case so we can get this PR on track quickly. Is this something you would agree upon?

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 28685 lines exceeds the maximum allowed for the inline comments feature.

@ja0nz
Copy link
Contributor Author

ja0nz commented Nov 13, 2021

Just out of curiosity I fixed some minor build related bugs in 22a1e46

And now the zero-install build runs smoothly. Have a look:
https://github.com/ja0nz/umbrella/tree/feature/yarnPnP

But the examples won't work with the new method. I will need to have a another look.

@postspectacular
Copy link
Member

postspectacular commented Nov 13, 2021

Thanks so much for the new detailed info! Personally I've got no issue to continue using "nodeLinker"/node_modules, especially if the alternative (aka PnP) causes more issues/friction with the examples and existing tooling and (to me) still offers no discernible real gains/benefits... I was mainly asking, because I (wrongly) thought yarn v2 onwards completely phased out node_modules and considering my negative experience with most JS tooling, this seemed a rather harsh decision and made me feel to rather wait a while/year longer for things to mature... So please do not worry about the PnP further (for now)...

The Lerna issue, however, is much more pressing to explore and I'm 110% happy to keep using Lerna for the foreseeable future! None of you will need to though, after your PR has been merged. But for releases I'm not aware of any other comparable solution - Lerna's (builtin, no extra plugins!) release handling of all these package is the no.1 killer feature I just cannot miss!

@ja0nz
Copy link
Contributor Author

ja0nz commented Nov 14, 2021

Aww, I think we'll get stuck here for a while:) Seems like yarn2 and lerna simply don't fit together. There's a 2 year open PR lerna/lerna#2450 for adding the workspace: resolution protocol which yarn workspaces rely upon. It does not even help to scope lerna towards packages/* (<- as they don't embody any workspace references yet).

In search of a more atomic publishing solution I got across two other big projects which tried/got rid of lerna (storybookjs/storybook#15413, ianstormtaylor/slate#4417). Most promising solutions resolves around atlassian's changesets. But sure enough finding a replacement will take a bit longer.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 28686 lines exceeds the maximum allowed for the inline comments feature.

@postspectacular
Copy link
Member

Thank you - again - for further probing/research! Really, much appreciated! I'm thinking (already did so a few times in the past) it might be easier & more worthwhile to write a custom publish script, rather than trying to wait or shoehorn some overblown/semi-incompatible solutions to work together and make everything super brittle as a result. It probably will take me a few days to look into that, but IMHO all the key ingredients are already provided by various umbrella packages. Another potential side effect would be that I can also improve the changelog files - and we could really say good bye to lerna rather sooner than later...

instead of forking out an bash script (with yarn exec)
the testrunner can be called directly with NODE_OPTIONS

- no cross env needed
Since Yarn2 there is a portable shell build in.
https://dev.to/arcanis/introducing-yarn-2-4eh1#normalized-shell
@postspectacular
Copy link
Member

Btw. @ja0nz - congrats on getting the CI working again! :)

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 28698 lines exceeds the maximum allowed for the inline comments feature.

@ja0nz
Copy link
Contributor Author

ja0nz commented Nov 15, 2021

I'm glad you still wanna follow this change (without lerna)! Happily awaiting the kind of publishing workflow you will come up:)

Just my two cents: After looking around a bit I kinda like the workflow around GH actions laid out here: https://github.com/azu/monorepo-release-changesets#an-example-of-work-flow
Mainly because changes are pretty descriptive for consumers (https://github.com/azu/monorepo-release-changesets/pulls?q=is%3Apr+is%3Aclosed)

But most likely you will come up with a lean solution based on @thi.ng. If there's something I may help out I will just let me know. Otherwise the last commit will be the last so far from my side.

@postspectacular
Copy link
Member

@ja0nz great news - I started working on the release tool this evening and very quickly made a lot of progress already: https://github.com/thi-ng/monopub - so far this new tool is also way more faster than what I've been used to from Lerna. I.e. On my MacBook Air M1 it takes just 1.062 secs to analyze all 7600+ commits, compute a new version for each package (figure out if needed) and generate & write categorized Markdown changelog files for 160 packages (full history, with more detail included than previously)! With Lerna I sometimes have to wait 20-30 secs for the getting that far... Also, the code so far is just around ~425 lines and I don't see this growing to more than 800-1000...

I've decided to create a new repo for this tool and added a little status section (if you want to follow along) to the readme. I will do some more work on this over coming days, but leaving this/your PR open for now, just in case I run into any unexpected show stoppers along the way... Thank you for all the great work on this so far though!

Re: your proposal re: changesets - I dunno, all of these solutions seem a lot more effort (and/or duplication of such). I actually do put quite a bit of effort into most/many commit messages and together with the Conventional Commits, all important information and details are already there... So thank you for looking into alternatives, but apologies, I can't see these working for me... 😇

...to refresh the yarn.lock index
Otherwise yarn tends to complain to run it first.
Quote:
"*": ["web_modules/.types/*"] is legacy and safe to remove
completely, there's no such thing as that directory anymore
(and never really was).
-- FredKSchott/snowpack#2255 (reply in thread)
Improving the overall build ergonomics
- introduced a tools workspaces
- imported it in all needed packages/examples
- inclusive project root
this commit reverts (partly) changes made in:
ef346d7

overall purpose is better testament ergonomics:
instead of having to pass NODE_OPTIONS with every invocation
having a binary to handle this for us.
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 29631 lines exceeds the maximum allowed for the inline comments feature.

@ja0nz
Copy link
Contributor Author

ja0nz commented Nov 17, 2021

Happy to see your progress with monopub🚀 I never thought this would mature so fast.

Well, I announced "the last commit" earlier but as time passed I accumulated some more commits I would like to add. Mostly some non functional cleanups like removing obsolete .gitignores... Nothing spectacular.

But there is one particular commit which I would like to address a bit more: bf7a404
This commit introduces a new tools module in the workspace which just exposes some binaries to the whole monorepo.
I prefixed those binaries with the tools: prefix just to make its usage more obvious.

So basically this line:
"doc:stats": "../../scripts/node-esm ../../tools/src/module-stats.ts"
can now be expressed as:
"doc:stats": "tools:module-stats"
Or this:
../../scripts/node-esm -> tools:node-esm

The naming and location might be debatable but overall the package.jsons looks somewhat cleaner now.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 29631 lines exceeds the maximum allowed for the inline comments feature.

@postspectacular
Copy link
Member

Yes, @ja0nz - this is all amazing (your updates/improvements) and the tools module is a great idea too! Will merge it all in a moment... Thank you SO much!

re: https://thi.ng/monopub - I'm too v. surprised how quickly this came together and it's actually already pretty flexible & nice to use. The CLI framework came from another tool I've been working on, but will eventually be finding its way back into umbrella (in parts, at least).

I'm now pretty much done for the first pass and have done some test releases with a dummy monorepo. So once your PR is merged I will do some further testing and then hopefully release new versions ASAP...

@postspectacular
Copy link
Member

postspectacular commented Nov 17, 2021

@ja0nz - one more thing/question: I've just noticed the CI test workflow for the yarn3 version takes now 32-33 minutes to complete, whereas it previously was only ~17-20 mins... Can you think of a reason for this slowdown?

@ja0nz
Copy link
Contributor Author

ja0nz commented Nov 17, 2021

@ja0nz - one more thing/question: I've just noticed the CI test workflow for the yarn3 version takes now 32-33 minutes to complete, whereas it previously was only ~17-20 mins... Can you think of a reason for this slowdown?

I noticed that as well. It seems to me that on GH actions yarn falls back to single threaded only. Others observed this as well yarnpkg/berry#3512
I am not an expert in GH actions but I think I can cobble a solution together where only changed packages will be tested.

@ja0nz
Copy link
Contributor Author

ja0nz commented Nov 17, 2021

@postspectacular - quick question. I am running on linux and with every yarn install I get a plethora of info logs

➤ YN0076: │ esbuild-android-arm64@npm:0.13.13 The linux-x64 architecture is incompatible with this module, link skipped.
➤ YN0076: │ esbuild-darwin-64@npm:0.13.13 The linux-x64 architecture is incompatible with this module, link skipped.

Is that the same on Mac? Just curious

@postspectacular
Copy link
Member

@ja0nz - that's what I thought (single threaded on CI). Not to worry for now. I just pulled your latest changes locally and it's all working just fine! As for the esbuild warnings - getting these too on the Mac, believe that's just how esbuild (or it's binary installer) is configured. I always ignore them :)

Going to merge now, wish us luck! :)

@postspectacular postspectacular merged commit fc2d657 into thi-ng:develop Nov 17, 2021
@postspectacular
Copy link
Member

So it all almost went 100% according to plan and all packages have been updated/released, yay!

Since this was my first publish using yarn3, the only thing which threw a spanner in the works was not having a valid NPM auth token (it's seemingly stored somewhere else now...) But all is good & thanks again!

One more (quite important) favor to ask for the future:

When you're using the conventional commits format and specify a "context" inside the parentheses, then please only use valid package names, not "yarn3", "snowpack" or other random stuff. If the commit is not primarily to do with an actual package, then please only use the more general format, i.e. build: yada instead of build(whatever-random): yada The reason for this is a whole bunch of other tooling is depending on this format (incl. the thi.ng website generator and the new monopub release tool), all of which are expecting these names inside the parentheses to be package names and they're now messing up... not a big deal (yet), but I will have to emphasize this a bit more in the contributing.md guide. Thanks for understanding! :)

@ja0nz
Copy link
Contributor Author

ja0nz commented Nov 18, 2021

Thanks for the valuable feedback. I will work on my commit message discipline🙏

Glad you "late night solved" the NPM thing! Either way allow me to quickly share how I solved it declarative just for the sake of exchanging knowledge😬

  1. I use a plain gitignored .env file:
# .env
NPM_TOKEN=npm_XXX
  1. I put the env straight into the yarnrc.yml:
# .yarnrc.yml

# Ensure (scoped) packages are published public; get rid of '--access public'
# https://yarnpkg.com/cli/npm/publish
npmPublishAccess: public

npmRegistries:
  //registry.npmjs.org:
    npmAuthToken: "${NPM_TOKEN}"

This approach works beautiful in conjunction with https://direnv.net/
3. My NOT gitignored .envrc file which pulls everything into place:

# .envrc
dotenv_if_exists .env
# add the ./scripts on PATH
PATH_add scripts
export ROOT=$(pwd)

@ja0nz ja0nz deleted the feature/yarn3 branch November 18, 2021 12:09
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.

None yet

3 participants