front: import osrd-ui #10711
Open
front: import osrd-ui #10711
+87,167
−10,891
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emersion
force-pushed
the
emr/monorepo
branch
5 times, most recently
from
98352b2 to
baf5d24
Compare
emersion
force-pushed
the
emr/monorepo
branch
from
b3c8d8d to
8dfca02
Compare
github-actions
bot
added
area:ci
emersion
force-pushed
the
emr/monorepo
branch
4 times, most recently
from
808b7bd to
04562bb
Compare
emersion
force-pushed
the
emr/monorepo
branch
4 times, most recently
from
f951b2c to
0fca49a
Compare
emersion
force-pushed
the
emr/monorepo
branch
4 times, most recently
from
a3f7fc5 to
5e3eba0
Compare
Otherwise these will blow up with out-of-memory errors when a new ui/ directory is imported in the next commit. Signed-off-by: Simon Ser <[email protected]>
This is the osrd-ui tree imported from tag 0.0.68, without any
change:
git clone --branch 0.0.68 https://github.com/OpenRailAssociation/osrd-ui.git ui
rm -rf ui/.git
Git history up to this state is available at:
https://github.com/OpenRailAssociation/osrd-ui/
The author list has been generated with:
git log --format='%aN <%aE>' | sort | uniq
With the following .mailmap file:
https://paste.sr.ht/~emersion/6cedd39f2935f6fcd3854eff1ce759c037965003
Signed-off-by: Simon Ser <[email protected]>
Co-authored-by: Achraf Mohyeddine <[email protected]>
Co-authored-by: Alexis Jacomy <[email protected]>
Co-authored-by: Alice Khoudli <[email protected]>
Co-authored-by: Benoit Simard <[email protected]>
Co-authored-by: Chaka NGAMENI NJINEH <[email protected]>
Co-authored-by: Clara Ni <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Egor Berezovskiy <[email protected]>
Co-authored-by: Élyse Viard <[email protected]>
Co-authored-by: Ethan Perruzza <[email protected]>
Co-authored-by: Florian Amsallem <[email protected]>
Co-authored-by: Gaël Haméon <[email protected]>
Co-authored-by: Ian Turner <[email protected]>
Co-authored-by: Mathieu <[email protected]>
Co-authored-by: Math_R_ <[email protected]>
Co-authored-by: Nicolas Wurtz <[email protected]>
Co-authored-by: romainvalls <[email protected]>
Co-authored-by: rostom boudi <[email protected]>
Co-authored-by: SarahBellaha <[email protected]>
Co-authored-by: Serge Croisé <[email protected]>
Co-authored-by: SharglutDev <[email protected]>
Co-authored-by: Simon Ser <[email protected]>
Co-authored-by: theocrsb <[email protected]>
Co-authored-by: Theo Macron <[email protected]>
Co-authored-by: thibautsailly <[email protected]>
Co-authored-by: Uriel-Sautron <[email protected]>
Co-authored-by: Valentin Chanas <[email protected]>
Co-authored-by: Victor "multun" Collod <[email protected]>
Co-authored-by: Yohh <[email protected]>
Co-authored-by: Younes Khoudli <[email protected]>
osrd-ui's root package.json is moved to ui/base/ because nested workspaces don't work with NPM: npm/cli#4774 rollup-base.config.js needs to be adjusted because node_modules now lives one directory higher and dist directories live one directory deeper. The build, test and lint scripts are adjusted to execute tasks for both osrd and osrd-ui. The full front source code is copied over in the Dockerfile before installing dependencies, because NPM needs to look at all workspace's package.json files to install dependencies. eslint-plugin-storybook's TypeScript version needs to be overridden because osrd uses 5.8 an the plugin asks for <5.8: storybookjs/eslint-plugin-storybook#193 Signed-off-by: Simon Ser <[email protected]>
Since front includes osrd-ui via NPM workspaces, dependabot will upgrade osrd-ui dependencies. We only need to slightly adjust group patterns to include everything missing from osrd-ui's configuration file. Signed-off-by: Simon Ser <[email protected]>
File paths need to be slightly adjusted. requirements.txt is moved to the root of ui-icons so that it can more easily be run locally. Signed-off-by: Simon Ser <[email protected]>
We need to strip the "v" prefix from Git tag names, and adjust file paths a bit. Signed-off-by: Simon Ser <[email protected]>
The build script is run for all workspaces, but not the root workspace. The deployment repository now needs to be explicitly specified. Signed-off-by: Simon Ser <[email protected]>
All of these are already covered by the osrd monorepo. Signed-off-by: Simon Ser <[email protected]>
When starting the front dev container, osrd-ui may not have been built yet, or might be outdated. Start a build of osrd-ui components which are used in osrd in parallel with vite. Avoid building osrd-ui's storybook, because it takes quite a while and it's unnecessary. Signed-off-by: Simon Ser <[email protected]>
osrd-ui needs to be built before starting the frontend. The "storybook" script is no more, instead users can cd into the storybook directory (just like all other sub-packages). Signed-off-by: Simon Ser <[email protected]>
There was no hint to indicate how one would start Docker Compose to work on the frontend. Signed-off-by: Simon Ser <[email protected]>
Unfortunately, ESLint's cache is pretty bare-bones and doesn't invalidate dependent files. For example, if a warning appears in foo.ts but is caused by a mistake in bar.ts, fixing bar.ts will not cause ESLint to re-check foo.ts and the warning will linger. I've seen multiple occurences of this happening. Deleting the ESLint cache directory and restarting ESLint helps, but it pretty cumbersome. Slow checks are better than invalid/stale warnings, so let's switch back to no cache. See the typescript-eslint docs, which also recommend to disable the cache: https://typescript-eslint.io/troubleshooting/faqs/eslint/#can-i-use-eslints---cache-with-typescript-eslint Signed-off-by: Simon Ser <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits.
TODO:
Closes: OpenRailAssociation/osrd-ui#451
Design decisions
Why move
front/ui/package.jsontofront/ui/base/package.json?NPM doesn't correctly handle nested sub-packages (
front/ui/andfront/ui/storybook/, for instance). It doesn't install dev dependencies of nested sub-packages (storybook dependencies are missing).Why keep
front/ui/base/package.json?front/ui/contains config files for rollup, ESLint, postcss used by all ui-* sub-packages. We need to specify dependencies for these somewhere.We could specify them in
front/package.json, but that would mix up osrd and osrd-ui deps. Keeping shared osrd-ui deps insidefront/ui/base/package.jsonmakes it clearer what's required by osrd itself and what's required by osrd-ui.Should we build osrd-ui when running
npm run build?Another possible solution would've been to only build osrd when running
npm run build, and require users to runnpm run build --workspacesto build osrd-ui. The upside would be that less work is performed when only touching osrd.However, this would make it so
npm run buildwould fail if osrd-ui hasn't been built before, or has an outdated build. Also, building osrd-ui requires less changes to the CI and tooling.There are ESLint errors when I start the front flavor of the Compose setup
This is due to osrd-ui and osrd being built in parallel when the container starts, and ESLint doesn't notice when osrd-ui has been built. Indeed, ESLint doesn't install a watcher on files in
node_modules/, which osrd-ui deps point to.To fix, run
npm run build --workspacesbefore starting the container, or restart the container (either should work).How will we merge this?
We will need to coordinate a bit to not loose anything in osrd-ui. Easiest would be to get this reviewed and ready to merge, freeze osrd-ui, release a new osrd-ui version and bump osrd to use that new version (if there are breaking API changes), adjust the "import osrd-ui" commit to import that version, and merge this PR.
Once that's done, all opened osrd-ui PRs will need to be manually migrated to the osrd repository.
Why
front/ui/instead offront/osrd-ui/?Because "osrd" is already in the name of this repo ¯\_(ツ)_/¯