Only update the details view when data has changed

[robertbrignull]

The idea here is to only emit the onDidChangeTreeData event when data has really changed.

This means we can call setState more liberally, such as on all webview visibility changes, without worrying about the performance impact of re-rendering the details with with the same content.

This relies on the contract that data won't be mutated while reusing the same object/array. So it's the same rules that React uses. I believe we currently abide by these rules and it's unlikely we'd want to break them, due to the way we generate external API usages we always end up with a new array object.

I've added some tests for the behaviour being changed in this PR. We could easily add more tests for the data provider, but to stop this PR from growing in scope I've opened an issue to cover adding more tests.

Checklist

• CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
• Issues have been created for any UI or other user-facing changes made by this pull request.
• [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

[koesie10]

Would it make sense to enforce this in the callers by always using a ReadonlyArray?

[robertbrignull]

I don't think it'll be simple, so I'm not sure if the benefit is worth the inconvenience.

I gave it a quick try and the scope quickly grows and we need to mark more and more arrays as readonly. It could be helpful still, but I think it's a large change of direction so I don't feel comfortable making it here. Investigating this further in another issue could be good, and we could consider making other types like ExternalApiUsage immutable as well.

My understanding is that in typescript you can cast a mutable array to an immutable array, but not the other way around. This means if we just specify setState to take a readonly array it doesn't gain us anything because the caller is still holding a mutable reference. Therefore we have to declare it as readonly as early in the process as possible, but this ends up infecting more and more of the codebase that isn't directly related to the details panel.

[koesie10]

Thanks for exploring this path! It makes sense that there are too many places where we would need to make updates, and it probably doesn't really make a big difference in type safety anyway. It would just increase the number of places where we need to specify types.