-
Notifications
You must be signed in to change notification settings - Fork 1
refactor(monorepo): convert ui-core to ui monorepo #822
Conversation
2170030
to
7ccab06
Compare
Copied from slack:
I def. do not agree with that.. I just like a molecule is not an atom. I prefer to think in terms of chemistry/biology. Then this whole differentiation makes a lot of sense. But I also don't see the benefit of calling it ui-core over ui-atoms?
|
The discussion we had about this on slack is here: https://dhis2.slack.com/archives/CBM8LNEQM/p1583309236163900 I mentioned the following:
So the reason I mentioned that I feel like we have a lot of discussions about what to put where was based on our discussing these things in the following issues: There is quite a bit of discussion on component organisation there. @varl mentioned:
And that is a clear definition. Yet somehow we still end up discussing these things a lot. So my question is, with this definition why do we still discuss where to put our components so much? And would this PR resolve that? (Ideally I think we would have a way of organising our components that is clear to us and our users. So that neither groups have to do too much thinking about what belongs/can be found where.) |
Problem 1: What goes where (core vs. widgets)
I do not see a lot of discussions about what an Atom is, everyone seems to have an OK grasp about what an atom is, so that is not a problem we are having. Molecules and Organisms have the definitions:
Relatively simple vs. relatively complex is hard to define. Right now we have "relatively simple" components in ui-core and "relatively complex" components in ui-widgets, and this creates a lot of back and forth. The idea is to move them all to widgets and removing the "relatively" from the mix and make it clear that if it's not an atom, it's a widget. Problem 2: Components that depend on data from DHIS2 APIThe Header Bar (at least) has a prop-driven version and a connected version. I think @Mohammer5's suggestion about having a separate DHIS2-layer lib (let's call it Problem 3: Dependency managementI still maintain that it is imperative for the long-term health of the UI system to have zero dependencies for the core components. I realize that we have a real need to be able to use dependencies, so the compromise is to allow dependencies in widgets (i18n, popper, etc.) and ban them in core. Problem 4: Developer convenienceWe can move components from core to widgets and maintain a proper split between all the UI libraries, and the drawback is when developing a molecule that needs new atoms in core, then they first have be merged to core or the developer has to link the libraries locally. Matters are made worse if the dev is working on a form component that needs something in widgets and that triggers work in core. In a monorepo we can have all the code organized properly in a single PR, and the developer is not "held back" by either local development tricks like links nor waiting for a PR. Problem 5: How can I find the component I need?
This PR also introduces a meta-package called
Problem 6: Version consistencyRight now, consolidating the UI-core versions across Apps, App Shell, ui-forms, ui-widgets, etc. can be tricky. We do some tricks to make ui-core external and use it as a peer dependency, but if someone has it as a straight dep there is a possibility that ui-core becomes out of sync between app and ui-widgets, and can result in unpredictable behavior. If we at least lock the versions together in a monorepo, then it becomes easier to debug and resolve those errors. |
I personally think heading this direction makes sense, and fully agree with the motivation given by @varl. Some ui-forms related notes:
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
bfc87c0
to
cc33470
Compare
Still work to be done, but I've reorganized the stories as an illustration: https://deploy-preview-822--dhis2-ui-core.netlify.com/demo/?path=/story/component-core-card--default |
git-subtree-dir: packages/widgets git-subtree-split: 70b0228a51d0337192ff2834d0cc0dac7ce78a59
BREAKING CHANGE: Rename the Constrictor component to Box, which is shorter and thus easier to type. This also expands the capabilities of Box to make it more Box-like.
BREAKING CHANGE: Replace SwitchGroupField, RadioGroupField, CheckboxGroupField with ToggleGroupField. BREAKING CHANGE: Replace SwitchGroup, RadioGroup, CheckboxGroup with ToggleGroup.
This extracts the UI constants to its own package, @dhis2/ui-constants, that includes shared constants for building UI. BREAKING CHANGE: The exports: colors, theme, layers, spacers, spacersNum, and elevations, have been moved from @dhis2/ui-core to @dhis2/ui-constants for better code reuse.
OK, this way of merging the repos is hard to maintain when it's WIP. Any change in ui-core is nigh impossible to rebase. I found a different strategy with subtree, that allows us to pull in changes in ui-core and ui-widgets into a new tree and rebase new changes on that. Superseded by: dhis2/ui#2 |
This is based on the proposal in https://github.com/varl/ui-merge-proposal, but it does the bare minimum as a first pass.
Right now this only includes two packages:
@dhis2/ui
@dhis2/ui-core
Required next steps:
@dhis2/ui-widgets
@dhis2/ui-forms
Optional steps:
Caveat
Yarn 1.19.2 has a bug in the Workspace feature causing
yarn install
to fail, so it is recommended to runyarn policies set-version 1.19.1
.The 300+ commits is not a mistake, it is done that way to preserve the entire Git history for the merged repository (e.g. all of the history for ui-widgets is preserved when merging it into ui-core).
Addedum: I squashed the history for now as it makes it easier to rebase on master