VIDSOL-281: refactor app config context for scoped reactivity and stable rests #251
Conversation
johnny-quesada-developer
changed the base branch from
main
to
johnny/vidsol-280-unit-test-fixes-and-refinements
johnny-quesada-developer
force-pushed
the
johnny/vidsol-VIDSOL-281-refactor-AppConfigContext-for-scoped-reactivity-and-stable-rests
branch
from
a4c1660 to
57d7614
Compare
johnny-quesada-developer
force-pushed
the
johnny/vidsol-280-unit-test-fixes-and-refinements
branch
from
a0d05c8 to
6d8ac7f
Compare
johnny-quesada-developer
force-pushed
the
johnny/vidsol-VIDSOL-281-refactor-AppConfigContext-for-scoped-reactivity-and-stable-rests
branch
from
57d7614 to
b10a7fc
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 83 out of 84 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
frontend/src/components/HiddenParticipantsTile/HiddenParticipantsTile.spec.tsx
Outdated
Show resolved
Hide resolved
johnny-quesada-developer
force-pushed
the
johnny/vidsol-VIDSOL-281-refactor-AppConfigContext-for-scoped-reactivity-and-stable-rests
branch
4 times, most recently
from
77170d3 to
55e2440
Compare
There was a problem hiding this comment.
Amazing work! Nice refactor to make testing/Contexts more readable and stable.
PD: There are missing changes regarding ../../../hooks/ I suggest changing it in another PR.
For example, changing:
import useIsSmallViewport from '../../../hooks/useIsSmallViewport';
johnny-quesada-developer
force-pushed
the
johnny/vidsol-280-unit-test-fixes-and-refinements
branch
from
6d8ac7f to
d3f10b6
Compare
- using granular context for app configuration... avoid re-renders and isolate logic - adding reusable provider wrappers for unit test - removing a bunch of mocks in our unit test... mocking our own code downgrades the quality of the test
johnny-quesada-developer
force-pushed
the
johnny/vidsol-VIDSOL-281-refactor-AppConfigContext-for-scoped-reactivity-and-stable-rests
branch
from
55e2440 to
74ac49d
Compare
|
| }); | ||
| }); | ||
|
|
||
| function render(ui: ReactElement, options?: SessionProviderWrapperOptions) { |
There was a problem hiding this comment.
could you please help explain if there is any particular reason (besides personal preference) to use the function notation instead of the arrow function notation we've been using throughout the project? I'd prefer to stick to the arrow notation please as to remain consistent; otherwise, we'll have to go in and change everything
There was a problem hiding this comment.
Hi of course @behei-vonage ,
Actually, I didn’t use the function notation out of personal preference, but because of hoisting. I’m declaring the function as a utility at the end of the file, but I want it to be available everywhere within the file. With an arrow function, I can’t guarantee that it will always be available, but with a regular function declaration, I can. Using the function keyword is more convenient in this case because I don’t even need to worry about where the render call is or anything like that.
With function key:
With arrow function:
There was a problem hiding this comment.
thanks for the explanation, @johnny-quesada-developer 🙏
in that case, will ensuring that the function is declared before its used be sufficient? I think it's important to stick to a particular way in a reference app
What is this PR doing?
This PR refactors the AppConfigContext to isolate updates and stop global re-renders.
It forces components to subscribe only to the specific configuration keys they depend on, eliminating unnecessary re-renders and improving overall performance.
It also introduces the isAppConfigLoaded flag to stabilize initialization and prevent UI flickers.
Additionally, it removes excessive mocking from our test suite to improve reliability and ensure tests reflect real behavior.
This change lays the groundwork for future low-code/no-code configuration flows, where the AppConfigContext will become more dynamic and require granular state updates.
How should this be manually tested?
What are the relevant tickets?
A maintainer will add this ticket number.
Resolves VIDSOL-281
Checklist
[x] Branch is based on
develop(notmain).[ ] Resolves a
Known Issue.[ ] If yes, did you remove the item from the
docs/KNOWN_ISSUES.md?[ ] Resolves an item reported in
Issues.If yes, which issue? Issue Number?