Module augmentation prefactorings #1616
Conversation
…as an implementation detail
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Ignored Deployments
|
|
@marcbouchenoire Do you have an idea what can cause the alternative ordering of those inbox notifications? Are we maybe missing an explicit sort on the backend side? |
@nvie Maybe the non-deterministic nature is due to timing? In the failing test I see the dummy notifications' timestamps slightly different, which makes sense since they're created one after another (2024-05-16T22:12:53.561Z vs 2024-05-16T22:12:53.562Z), maybe there are cases/cycles where the timestamps align and the notifications get sorted differently because of that? |
nvie
force-pushed
the
module-augmentation-prefactorings
branch
from
752c82b
to
911c5af
Compare
nvie
force-pushed
the
module-augmentation-prefactorings
branch
from
406e984
to
9c2108b
Compare
| /** | ||
| * @private | ||
| * | ||
| * This is an internal API, use `createRoomContext` instead. | ||
| */ | ||
| export function useRoomContextBundleOrNull() { | ||
| const client = useClientOrNull(); | ||
| return client === null ? null : getOrCreateRoomContextBundle(client); | ||
| const room = useRoomOrNull(); |
There was a problem hiding this comment.
The key line for this fix. useRoomContextBundleOrNull() isn't really dependent on having a room in order to create the bundle, but… the way the useSharedBundle() function works is that it only expects this function to return a result if a RoomProvider context is active. This was a tricky one! Super glad you discovered this, @marcbouchenoire.
Next week, I'll ensure that the E2E page/test will also capture this edge case before I continue to refactor more.
nvie
force-pushed
the
module-augmentation-prefactorings
branch
from
9c2108b
to
1d2e833
Compare
@marcbouchenoire Yeah, maybe. I just added ebd6ab2 which sorts the arrays before comparing them. I really wish there was a native matcher for this in Jest. |
nvie
force-pushed
the
module-augmentation-prefactorings
branch
from
29bbb87
to
ebd6ab2
Compare
This preps the Liveblocks context module by removing all remaining closures from the bundle factory, move all closed-over hooks to the top level and parameterize them all with a client instance. This will pave the way for the addition of the new top-level hooks.
Note
💡 Best reviewed commit-by-commit to see each refactoring step!
This PR should be possible to be merged as-is, because no behavioral changes are expected here.
@marcbouchenoire I'd appreciate if you could manually double-check this work, by running a couple smoke tests manually here. I have run all the unit tests, but this particular part of the code base is not 100% covered.
I'll make a similar changes to the Room context factory tomorrow — but there are a lot more of those there, so I first want to get you on board with this subset.