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

Monorepo ADR #1191

Merged
merged 3 commits into from
Dec 19, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions adrs/0002-monorepo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# [Transition Stacks to a monorepo]

**Date:** 2022-11-28

**Related PR:** #1191

## Issue

The Stacks repo is growing in complexity and will soon require many different dependencies that are optional for most consumers.

## Decision

Despite shipping many different concepts (atomic classes, CSS components, JS components), the interdependency chains are relatively linear. This means that we can fairly trivially split the repo into smaller portions.

Our current codebase could be split like so:
- Atomic classes (no dependencies)
- CSS Components (depends on Atomic components)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to keep CSS components and JS components in the same package (in the spirit of a true component based architecture - see also folder-by-feature vs folder-by-type). I would even go a step further and create folders for individual components where we keep style (less), behavior (stimulus - when applicable) and component test files (tbd - perhaps wtr + testing-library) together.
In an ideal scenario even the "docs" would be in the same component folder but I understand that it won't be an easy thing to achieve given our current setup. I would be ok to settle for docs in a separate package (revaluate perhaps at a later stage).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason I had suggested we separate the two is two-fold:

  1. If you only need the Stacks classes and not the JS components, you don't have to pull them in
  2. We're looking to modernize our JS component story, which means potentially having more than one supported component library (e.g. Stimulus + Web Components / React / Svelte / whatever), at least for a short while

That being said, I'm open to discussion on this point. I'm very much interested in hanging with the team, hashing out the pros / cons and coming to a hard decision. I'll schedule something for us for before the end of the calendar year.

Copy link
Contributor

Choose a reason for hiding this comment

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

Point 1 could be achieved even if we ship a single npm "stacks-components" package.
We will still have a way for consumers to opt in for css only if they want to I suppose.

The reason why I am suggesting for the component styles to go hand in hand with Stimulus is because I don't see us reusing any of that source code (Stimulus and Less) when we will build web components (or anything other component library).
My thinking as of now is that new components will depend exclusively on the atomic classes package and work in any context without depending on global styles being applied to a page or not.

I believe styles will have to be rewritten but I might be miscalculating things. Discovery will certainly help.
In case we will reuse the same component styles we have today in Stacks with the new components then I would understand why we would keep components styles in a separate package.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's just my ¢2, but I agree with Giamir that a single package for both CSS components and JS components makes the most sense.

On top of Giamir's points, it's plausible that we could convert a CSS component to a JS component at some point. Correct me if I'm wrong, but I imagine we'd need to deprecate the component in the CSS package and port it to the JS component package. That seems like it would be messy and a single Stacks components package makes more sense to me intuitively.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is the only pending topic before we could merge this ADR too. 🙂

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is actually something I feel strongly about and would be better suited to discuss over a hangout. In the meanwhile, I'm going to go ahead and merge as-is, since that line is just a suggestion and implementation can differ from what I have here.

We can always come back and update this later. The PR for that will serve as its own little mini ADR, since it'll contain the reasoning for the change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have mixed feelings about this PR being merged in without reaching an agreement on this point first. This is primarily because what we merge we consider automatically as approved (we don't have a status flag).
Can I suggest to add a comment/line directly in the ADR content to remind ourselves that further discussion is required about packages organisation.
Don't get me wrong, I like to move things forward fast but I also think it's ok to slow down a bit when we cannot agree on something as a team. @b-kelly Can I leave this small adjustment with you together with scheduling a meeting for us to talk further about this topic when the time is right? Thanks and sorry if this request sounds a bit pedantic. 🙂

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@giamir I'll make an adjustment. I see this section as a "for example" section, not as something I was proposing in this PR. The wording could be alludes to that. Would it make more sense if I reworded it like:

For example, our current codebase could be split like so

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me. Thanks.
I guess the more important action item is to come together as a team, take a decision and retrospectively justify our reasoning in this ADR too (we are not in a rush though). 🙂

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wording tweaked in 84a917c

- JS Components (depends on Stimulus, Popper.js, CSS + Atomic components)
- Documentation site (depends on JS components, 11ty + plugins)
Copy link
Contributor

Choose a reason for hiding this comment

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

if we keep js and css components in the same package then this will depends on a "stacks-components" package and transitively on the atomic classes package (perhaps "stacks-base")


When the codebase expands in the future, we'll be able to add the following without worrying about consumers needing to import dependencies they don't need:
- Front-end component library or libraries (e.g. React, Web Components)
- Large, complex components (e.g. autocomplete)
- Extra build chain items, such as a [Style Dictionary](https://amzn.github.io/style-dictionary/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice that you mention a design tokens tool here - it might be something worth exploring in the future (especially if we would have to start supporting non-web platforms too). 🙂

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did some exploration on this during my personal "Learn, Share, Grow" day. Technically, we already support a "non-web" (non-CSS?) platform - Figma 😉


We'd also be able to fold the [Stacks-Icons](https://github.com/StackExchange/Stacks-Icons) and [Stacks-Utils](https://github.com/StackExchange/Stacks-Utils) repos into this main monorepo as well.
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this idea.


## Additional Info

On the technical front, we could manage the monorepo with [npm workspaces](https://docs.npmjs.com/cli/v9/using-npm/workspaces?v=true) or another dedicated tool such as [Rush](https://rushjs.io/pages/intro/welcome/), [Lerna](https://lerna.js.org/) or [Bit](https://bit.dev/).
Copy link
Contributor

Choose a reason for hiding this comment

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

I have had mixed experiences with npm workspaces in my previous team (e.g. poor support of selective deps resolutions, upgrade deps pain, etc...) but in the end it did its job and it's nice you don't need to install any extra tool. Workspaces in yarn is way more mature though (or at least it was when I evaluated both tools half a year ago or so - maybe npm caught up a bit in the meantime).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Definitely down to do some discovery on this. Ultimately, I don't really care what tool we use so long as it stays out of the devs' way.


In regards to changelog / release notes generation, we'd continue using Conventional Commits, but moving to use [release-please](https://github.com/googleapis/release-please) which [does support monorepos](https://github.com/googleapis/release-please/blob/main/docs/manifest-releaser.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have hands on experience with release-please but I am excited to try that out. I believe we should automatize our release process as much as possible (including publishing packages to npm).
I have used conventional commits and I really like it. We might be even able to enforce those conventions with commitlint and nudge people in the right direction with comittizen (in my previous project most teammates they defaulted to run npx cz instead of using the git cli to create commits). 🙂

Copy link
Collaborator Author

@b-kelly b-kelly Nov 28, 2022

Choose a reason for hiding this comment

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

I've played with release-please only a little, but I've been using commit linting (plus a GH action to enforce) over in Stacks-Editor for a while now to great success. It's something that we've unofficially adopted across all of the Stacks repos recently, but haven't yet had the opportunity to add formal enforcement mechanisms.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've used Commitizen in the past and appreciated it (but I'm open). If we used Commitizen, I'd suggest configuring it on the side of warn to start, because it can get too annoying to have it yell at you for trying to commit with a message like cleanup when you're gonna squash it anyways. I want to make sure our tooling resides in the Goldilocks zone of annoyingness.


For backwards compatibility, we can trivially create a `@stackoverflow/stacks` "meta-package" that simply imports, then re-exports the dependency packages.