Make InProgressMethods immutable so it's safer to use as react state

[robertbrignull]

A small modification that makes setPackagedMethods return a new object instead of mutating the existing object. The current code is correct, but I worry the chance of the class accidentally being used incorrectly is a bit too high.

The danger of having InProgressMethods be mutable is that we use it as react state, and react tracks state changes only by object identity. This means if a developer do something like:

setInProgressMethods((oldInProgressMethods) => {
    oldInProgressMethods.setPackagedMethods(packageName, methods);
    return oldInProgressMethods;
);

then react would not see that the object was modified and would not trigger a re-render of the component. When the next render happened for whatever other reason, the change in state would be rendered. This leads to bugs and unpredictable visual behaviour.

By making InProgressMethods immutable it means you don't need to know that you must call fromExisting first to create a new object, and it'll be harder to accidentally mutate an object being used as react state.

It's worth nothing we do also have this problem with other objects and also with arrays. This one stuck out to me because it's a custom object with an interface, so the mutation of lack of it is hidden from the developer. We may also want to look at ways of reducing the chance of this bug in other places. Maybe a CodeQL query?

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]

LGTM, thanks for making this safe to use in React state

[charisk]

Thanks!