flo-bit / youtube-party-dj

democratic youtube player, everyone can vote and add songs, uni project, made with uix
MIT License
9 stars 2 forks source link

Suggestions for improvement / type fixes #63

Closed benStre closed 2 weeks ago

benStre commented 2 weeks ago

Hello team youtube party dj,

Great app idea and implementation! Sorry for the inconveniences you had with UIX :)

There were some problems with reactive array sorting that should now be more or less fixed in the latest DATEX/UIX release. Liking a video sometimes still leads to an infinite loop of list re-rendering. This is probably still a bug on the UIX side - we will investigate this further an get back to you.

In this PR, I mostly added some type fixes. Now there should be less TypeScript errors in your project :)

For typing pointers correctly, you should always use the generic argument like this:

const searchResults = $$<Item[]>([]);

or

const searchResults = $$([] as Item[]);

Then you don't longer need the ts-ignore statements for things like items.$.map

It is also better to pass normal (reactive) objects to a component (item={item}) and not the reactive property helper object (item={item.$}).

In your <QueueItem> component, you fetched the current user again for each component instantiation (const userId = (await getUserId()).userId;). This can be optimized by only calling it once in the module root scope to prevent unnecessary remote function calls each time the list is updated.

flo-bit commented 2 weeks ago

interesting how you got it to work without all the .val things put randomly in there, I never got that to work without, even though it didn't really make sense to me, is that because we passed in the reactive property helper item={item.$}?

anyway, thanks for taking a look and also fixing some of our typescript mess :)

benStre commented 2 weeks ago

Yup, when using item.$, all the properties are Ref objects and you need to cancel out the $ to get the a actual value by using .val, so item.$.xyz.val is essentially the same as item.xyz.

When you need reactive UI, you can still use {item.$.xyz} in your JSX - but this is only necessary outside of always blocks.