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

refactor: use generator function for generic worker, computeds in stores, simplifications #2525

Merged
merged 4 commits into from
Dec 29, 2024

Conversation

ferferga
Copy link
Member

@ferferga ferferga commented Dec 3, 2024

This spawns the worker only when needed, avoiding the extra memory usage of keeping it running all the time

Also properly release the worker in JVirtual

TODO: Playback manager doesn't shuffle correctly with this Given the shuffling issue that was going on due to a race condition between the currentPlaybackUrl and the currentItem in playbackManager, I took the plunge to also do a refactor I was looking forward to do since some long time. Every commit has detailed explanations.

@jellyfin-bot jellyfin-bot added the vue Pull requests that edit or add Vue files label Dec 3, 2024
@jellyfin-bot
Copy link

jellyfin-bot commented Dec 3, 2024

Cloudflare Pages deployment

Latest commit e6f5a06
Status ✅ Deployed!
Preview URL https://8c0f1f29.jf-vue.pages.dev
Preview alias https://improve-generic-worker.jf-vue.pages.dev
Type 🔀 Preview

View build logs

This spawns the worker only when needed, avoiding the extra memory usage of keeping it running all the time

Also properly release the worker in JVirtual

Signed-off-by: Fernando Fernández <[email protected]>
Initially, when we were migrating to Vue 3, `value` was really annoying
to work with. However, as time has passed by, using `value` provided
a clear advantage over reactive: It was clear when a value was reactive,
when without it, it was not. Also, using TypeScript getters
meant we had to do some dirty workarounds (like verify in watchers
if the id of the item currently playing was the same after the current one
after shuffling). With computed, we can use the `previous` argument
of the callback and everything should be more performant (since everything
is cached and recalculated only on dependency changes, instead of on every access)

We could also refactor some properties to use VueUse's computedAsync (
like `currentPlaybackInfo` in `playbackManager`) and keep everything
consistent (so we don't end up with a mixed usage of computed and getters)
Also took some tips from https://www.totaltypescript.com/tsconfig-cheat-sheet
Not implemented any of the options mentioned, but also
it's worth looking to https://www.totaltypescript.com/typescript-is-coming-to-node-23
to prepare for future Node native TypeScript support.

Signed-off-by: Fernando Fernández <[email protected]>
This greatly simplifies the timing of the cleanups performed when the current user logs out of the application

Signed-off-by: Fernando Fernández <[email protected]>
@ferferga ferferga force-pushed the improve-generic-worker branch from a58c6a8 to e6f5a06 Compare December 29, 2024 02:20
@ferferga ferferga marked this pull request as ready for review December 29, 2024 02:20
@ferferga ferferga changed the title refactor: use generator function for generic worker refactor: use generator function for generic worker, computeds in stores, simplifications Dec 29, 2024
@ferferga ferferga enabled auto-merge (rebase) December 29, 2024 02:23
@ferferga ferferga merged commit 7a1d35c into master Dec 29, 2024
19 of 22 checks passed
@ferferga ferferga deleted the improve-generic-worker branch December 29, 2024 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vue Pull requests that edit or add Vue files
Projects
Development

Successfully merging this pull request may close these issues.

2 participants