1839191 - Stop using context in React components where it isn't relevant

Status: RESOLVED FIXED

(DevTools :: Debugger, task)

Description

[Alexandre Poirot [:ochameau]]

In some places, we are setting a context properties but that's unused.
In some other places, we pass a context, but the action isn't specific to a given page so we shouldn't do any context assertion (example project root).
Last but not least in many places we use context to cancel dispatching some actions if the React component migrated to a new context. But instead we could more simply check if the input argument is still valid. For example when selecting thread/frame we could check if it is still in the redux store... and valid.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1839191 - [devtools] Stop using context for project root actions.

This data isn't related to a particular thread.
We assume that the project directory root works across reload.

Also clarify that project root data is a SourceTreeItem uniquePath.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1839191 - [devtools] Remove unused context variables.

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1839191 - [devtools] Stop using context for select thread action.

This action doesn't really benefit from context assertions.
Instead we could only verify that the to-be-selected thread still exists.

In order to do that, the pause reducer now store the list of active threads.

Stop trying to select the top thread on startup early as the related target/thread
isn't registered yet.

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1839191 - [devtools] Stop passing context for expression actions which shouldn't be cancelled.

When adding, updating and deleting expressions, this is rather like a UI action
and isn't contextual to a given page. We shouldn't try to stop these actions.

Also move "deleteExpression" to non-thunk action and make "getMappedExpression"
an utility function instead of an action as it doesn't dispatch any nested action.

Update some jsdoc and simplify "addExpression".

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e5b0a42b554
[devtools] Stop using context for project root actions. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/ab974ebe59e5
[devtools] Remove unused context variables. r=devtools-reviewers,bomsy

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/1e5b0a42b554
https://hg.mozilla.org/mozilla-central/rev/ab974ebe59e5

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4651551a6ca0
[devtools] Stop using context for select thread action. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/efd8b9a86eb6
[devtools] Stop passing context for expression actions which shouldn't be cancelled. r=devtools-reviewers,bomsy

Comment 8

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/4651551a6ca0
https://hg.mozilla.org/mozilla-central/rev/efd8b9a86eb6

Comment 9

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1839191 - [devtools] Stop using context for selectSourceURL, setPendingSelectedLocation and clearSelectedLocation.

All these three actions aren't related to any thread/source and shouldn't be cancelled.

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1839191 - [devtools] Remove useless cx argument passed to clearPreview.

This is no longer an action but a custom method on Preview/index.js.
Also fix the missing clearPreview method passed manually from Popup to ExceptionPopup.

Comment 11

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1839191 - [devtools] Remove context from removeAllBreakpoints and toggleSourceMapIgnoreList actions.

These are rather a UI actions which shouldn't be dismissed.

Comment 12

[Alexandre Poirot [:ochameau]]

Let me open a cloned bug for the other patches as the merge day get closer.

Comment 13

[Phabricator Automation]

Comment on attachment 9341628 [details]
Bug 1839191 - [devtools] Stop using context for selectSourceURL, setPendingSelectedLocation and clearSelectedLocation.

Revision D182407 was moved to bug 1841041. Setting attachment 9341628 [details] to obsolete.

Comment 14

[Phabricator Automation]

Comment on attachment 9341629 [details]
Bug 1839191 - [devtools] Remove useless cx argument passed to clearPreview.

Revision D182408 was moved to bug 1841041. Setting attachment 9341629 [details] to obsolete.

Comment 15

[Phabricator Automation]

Comment on attachment 9341630 [details]
Bug 1839191 - [devtools] Remove context from removeAllBreakpoints and toggleSourceMapIgnoreList actions.

Revision D182409 was moved to bug 1841041. Setting attachment 9341630 [details] to obsolete.