Paths

Table of Contentst

Differential D9402

[lib] Handle `[leave/delete]ThreadActionTypes.success` in `reduceThreadActivity`
ClosedPublic
Actions

Authored by atul on Oct 6 2023, 11:25 AM.

Tags

None

Referenced Files

	F3373581: D9402.diff
	Tue, Nov 26, 10:28 AM

	Unknown Object (File)
	Tue, Nov 19, 5:44 PM

	Unknown Object (File)
	Wed, Oct 30, 3:35 AM

	Unknown Object (File)
	Mon, Oct 28, 8:18 PM

	Unknown Object (File)
	Oct 25 2024, 1:15 AM

	Unknown Object (File)
	Oct 18 2024, 9:42 PM

	Unknown Object (File)
	Oct 18 2024, 9:42 PM

	Unknown Object (File)
	Oct 18 2024, 9:42 PM

View All 57 Files

Subscribers

Details

Reviewers

ashoat
ginsu
tomek

Commits

rCOMM7c3352f44588: [lib] Handle `[leave/delete]ThreadActionTypes.success` in `reduceThreadActivity`

Summary

We want to remove entries from ThreadActivityStore once the threads have been removed.

Test Plan

Navigate to thread, log ThreadActivityStore, delete thread, log ThreadActivityStore.

Diff Detail

Repository

Branch

master

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Oct 6 2023, 11:25 AM

Herald added a subscriber: will. · View Herald TranscriptOct 6 2023, 11:25 AM

atul published this revision for review.Oct 6 2023, 11:25 AM

atul added inline comments.

lib/reducers/thread-activity-reducer.js
37–40	Maybe this check is superfluous and we should just let execution spill into the subsequent `for` loop? I guess this prevents `state` pointing to different object if there are no updates, but there will almost certainly be updates with `leaveThreadActionTypes.success` and `deleteThreadActionTypes.success`? Can remove this if preferred

Harbormaster completed remote builds in B23062: Diff 31752.Oct 6 2023, 11:43 AM

atul added a child revision: D9404: [web] Update `threadActivityStore[activeThread].lastNavigatedTo` state.Oct 6 2023, 12:38 PM

ashoat accepted this revision.Oct 9 2023, 1:59 PM

ashoat added inline comments.

lib/reducers/thread-activity-reducer.js
37–40	No it's better to keep this. If for instance there is a `useSelector(state => state.threadActivityStore)` (or a `createSelector` equivalent), they would be able to "short-circuit" and skip any changes here because you're returning the exact same `state` object. If you remove it here, that "short-circuit" will be gone

This revision is now accepted and ready to land.Oct 9 2023, 1:59 PM

rebase and land

This revision was landed with ongoing or failed builds.Oct 9 2023, 2:16 PM

Closed by commit rCOMM7c3352f44588: [lib] Handle `[leave/delete]ThreadActionTypes.success` in `reduceThreadActivity` (authored by atul). · Explain Why

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM7c3352f44588: [lib] Handle `[leave/delete]ThreadActionTypes.success` in `reduceThreadActivity`.

Harbormaster completed remote builds in B23130: Diff 31837.Oct 9 2023, 2:32 PM

Shouldn't we also handle processUpdatesActionType action?

atul mentioned this in D9453: [lib] Update `reduceThreadActivity` to handle `processUpdatesActionType` and others.Oct 11 2023, 1:27 PM

atul mentioned this in rCOMM4d26804581dc: [lib] Update `reduceThreadActivity` to handle `processUpdatesActionType` and….Oct 19 2023, 7:44 AM

Revision Contents
Changeset List

Path

Size

lib/

reducers/

thread-activity-reducer.js

24 lines

Diff 31752

lib/reducers/thread-activity-reducer.js

Loading...