Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

refactor(monorepo): convert ui-core to ui monorepo #822

Closed
wants to merge 47 commits into from
Closed

Conversation

varl
Copy link
Contributor

@varl varl commented Mar 4, 2020

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:

  • Fix Jest
  • Fix Cypress
  • Fix docs
  • Pull in @dhis2/ui-widgets
  • Pull in @dhis2/ui-forms
  • Move molecules out of core to widgets (enables translations, etc.)
  • Move dependencies out from core to widgets (enables dependencies)

Optional steps:

  • Reorganize structure of stories
  • Rewrite stories in SCF

Caveat

  • Yarn 1.19.2 has a bug in the Workspace feature causing yarn install to fail, so it is recommended to run yarn 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

@varl varl force-pushed the monorepo branch 2 times, most recently from 2170030 to 7ccab06 Compare March 4, 2020 06:31
@Mohammer5
Copy link
Contributor

Copied from slack:

and it's worse because an organism is also a molecule
it's just a large molecule
(by @varl)

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?

this is final, lthe benefit is not breaking the world by renaming ui-core to something else again
(response by @varl)

@ghost
Copy link

ghost commented Mar 4, 2020

The discussion we had about this on slack is here: https://dhis2.slack.com/archives/CBM8LNEQM/p1583309236163900

I mentioned the following:

@varl To me it feels like the core of the issue was unclarity about what to put where in our ecosystem. I.e., where do components with translations go, where does this go, where does that go. I think in the original issue, @Jan-Gerke Salomon's proposal seemed like a good solution to me because it attempted to clarify the distinction between our libs.With the monorepo I don't really see how this would help us distinguish better between our different libs and figure out what to put where. Maybe I'm overlooking something, let me know if I am.

Not saying that JG's proposal would be the only or exact way I'd advocate, but I did like the attempt at setting up clear boundaries between the different libs we have. That felt like the main problem to me.

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:

  • atoms => core
  • molecules/organisms => widgets

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.)

@varl
Copy link
Contributor Author

varl commented Mar 4, 2020

Problem 1: What goes where (core vs. widgets)

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?

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:

[...] Molecules are relatively simple groups of UI elements functioning together as a unit.
[...] Organisms are relatively complex UI components composed of groups of molecules and/or atoms and/or other organisms.

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 API

The 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 ui-data for now) that has the connected variant is interesting. It does open up for discussion about "what goes where" so I'd prefer that it just goes in widgets.

Problem 3: Dependency management

I 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 convenience

We 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?

(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.)

This PR also introduces a meta-package called @dhis2/ui that just re-exports all of our ui-* libraries under a single namespace, so a user can either install e.g. @dhis2/ui-core specifically, but more likely, the user will install @dhis2/ui and then use it:

import { Button } from '@dhis2/ui'
import { HeaderBar } from '@dhis2/ui'

Problem 6: Version consistency

Right 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.

@HendrikThePendric
Copy link
Contributor

HendrikThePendric commented Mar 4, 2020

I personally think heading this direction makes sense, and fully agree with the motivation given by @varl.

Some ui-forms related notes:

  • When pulling in ui-forms, the components need to be renamed. I believe we agreed on the Control suffix. So that'd mean Input should be renamed to InputControl etc.
  • In ui-forms we have three distinct entities we export: components, validators and transformers. I'm not quite sure what would be the best approach for exporting them from @dhis2/ui... We probably shouldn't mix these validators and transformers with the components (i.e. core+widgets+forms components) ...

@cypress
Copy link

cypress bot commented Mar 5, 2020



Test summary

274 0 0 0


Run details

Project ui-core
Status Passed
Commit 6bd9038
Started Mar 6, 2020 10:08 AM
Ended Mar 6, 2020 10:15 AM
Duration 06:52 💡
OS Linux Ubuntu Linux - 18.04
Browser Electron 78

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

@varl
Copy link
Contributor Author

varl commented Mar 6, 2020

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

varl added 24 commits March 6, 2020 11:04
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.
@varl
Copy link
Contributor Author

varl commented Mar 6, 2020

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

@varl varl closed this Mar 6, 2020
@varl varl deleted the monorepo branch March 6, 2020 12:31
@varl varl mentioned this pull request Mar 6, 2020
10 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants