eclipse-cdt-cloud / vscode-memory-inspector

vscode memory inspector
https://open-vsx.org/extension/eclipse-cdt/memory-inspector
Eclipse Public License 2.0
6 stars 10 forks source link

Proposal: Use React context instead of prop drilling #85

Open tortmayr opened 6 months ago

tortmayr commented 6 months ago

What it does

How to test

Full feature testing of the webview is required to ensure that everything works as before.

Review checklist

Reminder for reviewers

tortmayr commented 6 months ago

In general of course the use of context vs props is a compomise between encapsulation of components and duplication. However, as the React components of the memory view share large parts of state and props anyway, I lean towards using a React context here.

I agree. Regarding encapsulation: context sharing has to be explicitly enabled on a per-component basis. (Either by assigning the contextType property or the useContext hook). Currently only the options and the table widget use the shared context. Here as you already mentioned it makes sense because the share a very large part of the state anyways. If only a minor subset is required we can still fallback to props drilling to not expose the full state to the component. (e.g. How it is done for the MoreMemorySelect widget)