Add separate provider for context and pass as children

[TimothyAsirJeyasing]

#672

[SanjalKatiyar]

plz test the PR properly and attach screenshots...

[TimothyAsirJeyasing]

[SanjalKatiyar]

/lgtm

[SanjalKatiyar]

/approve cancel

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: TimothyAsirJeyasing
Once this PR has been reviewed and has the lgtm label, please ask for approval from sanjalkatiyar. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• packages/mco/OWNERS
• packages/odf/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[alfonsomthd]

@SanjalKatiyar is this approved?

[GowthamShanmugam]

won't this create a new object after every render? due to this TopologySearchContext.Provider will get new params after every render right?

[GowthamShanmugam]

I still have doubts about this PR, Whether the provider is a separate component does not matter, when the provider props change then It will re-render all of its children. It does not matter whether the child component is a consumer of the provider's property or not.

All we can do is pass the parent state to the provider instead of passing the direct value to avoid the request re-rendering.

[SanjalKatiyar]

I still have doubts about this PR, Whether the provider is a separate component does not matter, when the provider props change then It will re-render all of its children. It does not matter whether the child component is a consumer of the provider's property or not.

All we can do is pass the parent state to the provider instead of passing the direct value to avoid the request re-rendering.

{children} part of this snippet should be the one making the difference:

<TopologyDataContext.Provider value={value}>
    {children}
</TopologyDataContext.Provider>

^ this is something which can be tested and cross-verified by updating the state continuously and adding a logger in any of the consumer component.

[GowthamShanmugam]

I still have doubts about this PR, Whether the provider is a separate component does not matter, when the provider props change then It will re-render all of its children. It does not matter whether the child component is a consumer of the provider's property or not.
All we can do is pass the parent state to the provider instead of passing the direct value to avoid the request re-rendering.

{children} part of this snippet should be the one making the difference:

<TopologyDataContext.Provider value={value}>
    {children}
</TopologyDataContext.Provider>

^ this is something which can be tested and cross-verified by updating the state continuously and adding a logger in any of the consumer component.

ack

[GowthamShanmugam]

LGTM

[bipuladh]

@TimothyAsirJeyasing can we test this PR out and see if there is a difference in re-render amounts significantly?

[openshift-ci]

New changes are detected. LGTM label has been removed.

[TimothyAsirJeyasing]

The profile details are varying, However i could observe some small improvement statistically in 10 to 20 attempt.
Before

after:

[SanjalKatiyar]

In case there is any confusion on why this change was needed (or why GH issue was created), plz check:
https://codesandbox.io/p/sandbox/without-children-h3kmm5
https://codesandbox.io/p/sandbox/with-children-qn3tcr

^ above was the idea for requesting this change, it's more of a good-practice/correct-way to add context following the pattern shown in the shared sandbox.

[TimothyAsirJeyasing]

/test odf-console-e2e-aws

[TimothyAsirJeyasing]

/test odf-console-e2e-aws