Skip to content

Conversation

Kocal
Copy link
Member

@Kocal Kocal commented Nov 2, 2024

Q A
Bug fix? no
New feature? no
Issues Fix #...
License MIT

When working in UX Map, it was very painful to build or watch modifications on .ts files, I had investigate how the building process worked, and had to run Rollup like this, which is far from ideal (not documented and not obvious):

./node_modules/.bin/rollup -c --environment INPUT_FILE:src/Map/assets/src/abstract_map_controller.ts -w
./node_modules/.bin/rollup -c --environment INPUT_FILE:src/Map/src/Bridge/Google/assets/src/map_controller.ts -w
./node_modules/.bin/rollup -c --environment INPUT_FILE:src/Map/src/Bridge/Leaflet/assets/src/map_controller.ts -w

And I also felt sorry for @rrr63 when he worked on adding Polygons on Map (#2162)...

This PR improves the way developers will work on UX, and makes their lives easier.

Before:

  • yarn build compiled the assets from ALL packages, it was not possible to build packages from only one package (which is useful if you work on a single package)
  • no yarn watch
  • yarn test runned tests from ALL packages, like yarn build, it was not possible to run tests for only one package

Now:

  • at the project root, build/test/lint/format/check-lint/check-format scripts will run on all assets from all packages. And it will be faster than before, when processing was sequential, but now it's parallelized.
  • at a package root (ex: src/Map/assets), build/test/lint/format/check-lint/check-format scripts will run on all assets from this package only
  • build and watch scripts handles both TypeScript and CSS files in a single command

This is a first step to what we spoke about with @smnandre to write a contribution guide.
It is now much more easier and friendlier to tell a developer to run yarn watch inside a package root, instead of telling it to run ./node_modules/.bin/rollup -c --environment INPUT_FILE:src/Map/src/Bridge/Google/assets/src/map_controller.ts -w (and more, if you have multiple files).

image

@carsonbot carsonbot added DX Status: Needs Review Needs to be reviewed labels Nov 2, 2024
@Kocal Kocal force-pushed the dx-better-assets-scripts branch from acab864 to 750de7d Compare November 2, 2024 19:22
@Kocal Kocal force-pushed the dx-better-assets-scripts branch from 6ca2c64 to a850401 Compare November 2, 2024 19:27
@Kocal Kocal requested review from kbond, smnandre and WebMamba November 2, 2024 20:02
@smnandre
Copy link
Member

smnandre commented Nov 2, 2024

This is so grea! And it does run very quickly!

Congrats and thank you 🙇

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Nov 2, 2024
@Kocal Kocal force-pushed the dx-better-assets-scripts branch from 7ec5095 to b8dc5fd Compare November 2, 2024 23:00
@smnandre smnandre changed the title [DX] Abuse yarn workspaces and add scripts-per-package (build, watch, test, lint, ...) [DX] Add per-package Yarn scripts (build, watch, test, lint, ...) Nov 3, 2024
@Kocal Kocal merged commit 57f7314 into symfony:2.x Nov 3, 2024
63 checks passed
@Kocal Kocal deleted the dx-better-assets-scripts branch November 3, 2024 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants