coder/internal#904

this doesn't really need to be a hook imo. it should be a query function like we generally use, and it can takeuser as an argument.

I know, but IMO, it encapsulates the intention quite well and improve the readability. It makes easier for me to know where and for what things are being used for. Eg. It is clear for me for what the useQuery and usesAuthenticated are being used for.

Unfortunately, you can't use hooks inside of regular functions, but I'm happy to change the code to use any existent convention.

the entirety of this hook except forconst { user } = useAuthenticated(); can be encapsulated in a query options function tho! and that better matches how we do it everywhere else.

Unfortunately, you can't use hooks inside of regular functions

and this is part of why I am so against custom hooks in 90% of use cases. they're viral function coloring, and should be avoided unless they are actually necessary to encapsulate stateful logic. I don't think that's the case here, as all of the logic is independent of the state.

I think the new React docs on custom hooks are very insightful. Even they don't think people should really be making custom hooks most of the time!

https://react.dev/learn/reusing-logic-with-custom-hooks#there-is-more-than-one-way-to-do-it

I don't understand the need for thisdata wrapper. why not just export the function directly?

Because of somespyOn limitations 😢. I'm not able to make it work when doing the following:

import*asdatafrom"./data"spyOn(data,"createTask")

Cannot spy on export "createTask". Module namespace is not configurable in ESM. See: https://vitest.dev/guide/browser/#limitations

But it should be temporary, since the create task API should handle that.

this doesn't hand the case wherea.username === a.username

Not sure if I got it 🤔

I can't even blame you, writing sorting functions is really unintuitive. 😅

ifa.username === a.username anda.username === value you'll still return-1 when you should return 0. they'reboth the selected user, so you shouldn't be preferring one over the other. although I guess saying it out loud, that case probably won't ever come up except maybe in tests that don't generate unique usernames.

Would it be ok if we let this as it is?

this might be a nice opportunity to usestructuredClone to mask some noise:

There is an issue with that,structuredClone keep the read only structure causing an TS error.

Cannot assign to 'state' because it is a read-only property.

all the same applies here

FC?

also, seems like a good candidate for a general component. might be worth moving to components/.

seems like a good candidate for a general component.

Wdyt about waiting for more use cases using it?

I can't even blame you, writing sorting functions is really unintuitive. 😅

ifa.username === a.username anda.username === value you'll still return-1 when you should return 0. they'reboth the selected user, so you shouldn't be preferring one over the other. although I guess saying it out loud, that case probably won't ever come up except maybe in tests that don't generate unique usernames.