♻️(frontend) Refactor: merge cunningham - #284
Conversation
jbpenrath
left a comment
jbpenrath
left a comment
There was a problem hiding this comment.
About the storybook, there is a lot of work to merge both docs and remove deprecated doc proper to initial design system
| { | ||
| files: ["packages/ui-tokens/**/*.ts"], | ||
| rules: { | ||
| "@typescript-eslint/no-explicit-any": "off", |
There was a problem hiding this comment.
I'll enable this rule. Allow any anywhere is IMO the best way to get a repo with shitty typing. There is few exceptions and in this case, we should disable the rule locally.
There was a problem hiding this comment.
Done in b65355e — the rule is now enforced repo-wide. The ~70 existing usages were typed properly where possible (unknown plus narrow casts for the token trees), with scoped eslint-disable lines only where downshift/react-aria expose incompatible generics for the same object.
There was a problem hiding this comment.
Not in this PR but with all supply chain attacks we will have to setup a docker workflow for this repo.
There was a problem hiding this comment.
Agreed — keeping it out of this PR and tracking the Docker workflow as a follow-up.
There was a problem hiding this comment.
It is no longer referenced anywhere in the repo. A fresh recording will be done manually and committed separately — keeping the current file until then.
|
|
||
| ## Prerequisites | ||
|
|
||
| - Node 20 and Yarn 1.22.22; |
There was a problem hiding this comment.
Right moment to use the latest LTS node version (24)
There was a problem hiding this comment.
I would do it in a separate PR to reduce the time-to-merge for this one. just in case it breaks things.
There was a problem hiding this comment.
Leaving the toolchain on Node 20 for now — the bump to 24 deserves its own coordinated change (CI, volta, engines) outside this PR.
|
I feel we lose some override nope ? (I did not found data-grid override for example) |
Relocate the existing UI Kit sources without changing their content. Prepare the repository for the workspace-based package layout.
Configure Yarn workspaces, Turborepo, Changesets, and shared tooling. Declare the three packages and update the monorepo lockfile.
Import the Cunningham token generator, CLI, defaults, and test suite. Preserve the historical output format and cunningham binary name.
Import Cunningham components, styles, stories, documentation, and tests. Merge shared components, locales, providers, exports, icons, and themes.
Rewrite legacy package, style, font, and icon imports from an npx command. Keep preview as the default and support explicit write and check modes.
Describe the monorepo packages, install paths, and migration command. Move release guidance under docs and define independent package versioning.
Cache workspace dependencies and run package checks through Turbo. Publish the validated Storybook artifact to GitHub Pages from main.
Track the token library sources hidden by the generic lib ignore. Build Storybook through Turbo so workspace dependencies are ready first.
322c48d to
f290a55
Compare
The merged library exposes the UI Kit dropzone uploader as the single FileUploader, so the Cunningham form input was dead code kept out of the public API. Drop the component, its tokens, styles, translations and stories, and point the Forms examples at the public uploader.
The package README described the token engine as Cunningham's while the monorepo now owns it. Apply the review suggestion naming it the UI Kit design-token engine.
Review asked why the flag exists. Removing it makes vite wipe the tsc output (.js and .d.ts) that build-lib emits into dist/lib right before the bundle step, so document the constraint instead of dropping it.
The Storybook brand image uses the UI Kit logos, so the imported Cunningham logo and favicon were dead weight from the merge. Remove them along with the favicon link in the preview head.
Cunningham was published under the @openfun scope before moving to @gouvfr-lasuite, so applications that never upgraded still import the old packages. Map them to the same targets as their successors in specifiers, styles and package.json dependencies.
The Cunningham merge left two folder conventions side by side. Align on the UI Kit standard, kebab-case folders with CamelCase component files, so contributors stop guessing which casing a path uses. Storybook titles keep their display casing. The ui-kit form folder and the renamed forms folder still coexist; merging their contents is tracked as a follow-up.
Blanket-allowing any invites untyped code to spread, so enable the rule repo-wide and type the seventy existing usages: real types where they were cheap, unknown with narrow casts for the token trees, and scoped disables only where downshift and react-aria expose incompatible generics for the same object.
|
Review follow-up: all inline comments are addressed on the branch (see per-thread replies). Two larger items are deliberately deferred to keep this PR reviewable: the Docker supply-chain workflow and the Storybook docs merge/dedup of the deprecated initial-design-system pages — both to be tracked as dedicated follow-ups. Also included: the Cunningham FileUploader is now fully removed (8213a2a) since the UI Kit dropzone uploader is the single public one. |
The suite overrides were loaded with @use "suite" at the top of each component's _index.scss, so Sass emitted them before the base styles. Both target the same selectors with equal specificity, so the base declarations won the cascade and the overrides were dead: the DataGrid kept 3rem rows, visible row borders and the upstream header style instead of the ui-kit look. Splitting the base styles into _base.scss lets _index.scss load base then suite in cascade order. The !important on the datagrid header font-size is dropped so the header--size token can now apply.
The merged Storybook config disabled autodocs, which removed the Docs pages of every story tagged "autodocs" (FilePreview, FileIcon, ReleaseNoteModal among others); only hand-written MDX docs survived. Autodocs was off because the merged Components/Button title is claimed both by the Cunningham MDX docs page and by the tagged ui-kit stories, which makes story indexing fail. Dropping the redundant tag on the ui-kit Button stories resolves the clash, so autodocs can run in "tag" mode again as it did before the merge.
Set both package to their previous version + one increment.
210e78f to
590e5ca
Compare
Bumping ui-tokens to 3.1.0 broke the workspace link: ui-components still required 1.0.0, so Yarn stopped resolving it internally and Turbo lost the build ordering. The cunningham binary produced by the ui-tokens build was then missing, failing CI on build-theme.
The former ui-kit sass/fonts bundle shipped the Material Icons fonts, but its replacement only carries the text fonts, so icons silently render as blank glyphs after migration. Document the explicit /material-icons import; keeping it explicit lets applications serve the icon fonts from a CDN instead without shipping them twice.
Filter, Calendar, Loader and LeftPanel exist both as root components and as SVG icons. The import splitter routed any icon-manifest name to /icons, so a root import of these components was silently replaced by the icon of the same name. Route a name to /icons only when it is not also a root export, using a root-manifest generated at build time.
3 participants
Summary
This PR merges Cunningham into UI Kit and turns the repository into a Yarn
monorepo with three packages:
@gouvfr-lasuite/ui-components: all UI Kit and Cunningham components.@gouvfr-lasuite/ui-tokens: the token engine, themes and generators.@gouvfr-lasuite/ui-codemod: an npx-only migration CLI.It also consolidates the documentation into a single Storybook, adds manual
release documentation and deploys Storybook to GitHub Pages after merges to
main.Testing
yarn lintyarn buildyarn testyarn test:ctSTORYBOOK_BASE_PATH=/ui-kit yarn build-storybook