Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(workspace): stabilize development mode (VIV-1526) #1691

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

YonatanKra
Copy link
Contributor

@YonatanKra YonatanKra commented May 2, 2024

In this PR:

  • Setup docs tests
  • Address docs:serve race condition

In future PRs:

  • Handle build errors: an error in any build step should stop the process and raise an error in the browser
  • Debounce the chokdiar change handler
  • Handle renamed/delete folders
  • Multiple dependency support?

@YonatanKra YonatanKra changed the title dev(workspace): stabilize development mode (VIV-1526) chore(workspace): stabilize development mode (VIV-1526) May 2, 2024
Copy link

codecov bot commented May 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (d61b119) to head (d5b6683).
Report is 914 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##              main     #1691     +/-   ##
===========================================
  Coverage   100.00%   100.00%             
===========================================
  Files          123       328    +205     
  Lines         1562      5656   +4094     
  Branches       108       718    +610     
===========================================
+ Hits          1562      5656   +4094     
Flag Coverage Δ
unittests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@RichardHelm RichardHelm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the PR and one issue I found is that it is a lot slower for component changes (~3s vs ~13s). It seems that vite watch mode is much faster than doing a full build

@YonatanKra
Copy link
Contributor Author

YonatanKra commented May 8, 2024

I tested the PR and one issue I found is that it is a lot slower for component changes (~3s vs ~13s). It seems that vite watch mode is much faster than doing a full build

Thanks for the feedback. It's a preliminary solution that's supposed to actually allow a working dev environment. For me it is a big improvement than the unlimited time waiting for a crush due to the the race condition in the builds :)
So perf improvements can definitely be a part of this library's roadmap.
BTW - one such perf improvement can be splitting the components into separate buildable libraries and utilize NX cache to speed things up (an idea I've been pondering for a while now).
As I mentioned - right now the dev mode doesn't work for me. I don't care too much because I develop with Jest, but I figure it bothers the rest of the team and for this I care. If there's no problem with current state, then we can ditch this solution. I'm not attached to it :)
I just need a decision, because performance was not the first priority - the first priority was to stabilize the dev process.

@TaylorJ76
Copy link
Contributor

I tested the PR and one issue I found is that it is a lot slower for component changes (~3s vs ~13s). It seems that vite watch mode is much faster than doing a full build

Thanks for the feedback. It's a preliminary solution that's supposed to actually allow a working dev environment. For me it is a big improvement than the unlimited time waiting for a crush due to the the race condition in the builds :) So perf improvements can definitely be a part of this library's roadmap. BTW - one such perf improvement can be splitting the components into separate buildable libraries and utilize NX cache to speed things up (an idea I've been pondering for a while now). As I mentioned - right now the dev mode doesn't work for me. I don't care too much because I develop with Jest, but I figure it bothers the rest of the team and for this I care. If there's no problem with current state, then we can ditch this solution. I'm not attached to it :) I just need a decision, because performance was not the first priority - the first priority was to stabilize the dev process.

I feel this is definitely an improvement on what we currently have. I think we should iterate on this to find ways to improve performance, but I'm a lot happier with changes actually being reflected in the browser and not having to re-run the build etc.

@YonatanKra YonatanKra marked this pull request as draft May 9, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants