-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue 1350 - Multi-input callback with sync event handling #1385
Changes from 17 commits
a8cc367
074e58a
783953b
60e8508
a5cb50d
f795fdb
8b6ff26
588afa0
d32e88f
d52cee7
f9e9bdb
0dd6ce1
56cac61
2b8a2e3
8471e14
f4086c6
3c0b127
90697b1
d057cd7
6fe5040
ed20242
5988554
ae33983
9c87c66
49fb65e
eca7ed1
4588463
cd1abbf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ interface IStoreObserverState<TStore> { | |
export interface IStoreObserverDefinition<TStore> { | ||
observer: Observer<Store<TStore>>; | ||
inputs: string[] | ||
[key: string]: any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just some flexibility when creating observers |
||
} | ||
|
||
export default class StoreObserver<TStore> { | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import { IStoreObserverDefinition } from '../StoreObserver'; | ||
import { IStoreState } from '../store'; | ||
|
||
const updateTitle = (getState: () => IStoreState) => { | ||
const { | ||
config, | ||
isLoading | ||
} = getState(); | ||
|
||
const update_title = config?.update_title; | ||
|
||
if (!update_title) { | ||
return; | ||
} | ||
|
||
if (isLoading) { | ||
if (document.title !== update_title) { | ||
observer.title = document.title; | ||
document.title = update_title; | ||
} | ||
} else { | ||
if (document.title === update_title) { | ||
document.title = observer.title; | ||
} else { | ||
observer.title = document.title; | ||
} | ||
} | ||
}; | ||
|
||
const observer: IStoreObserverDefinition<IStoreState> = { | ||
inputs: ['isLoading'], | ||
mutationObserver: undefined, | ||
observer: ({ | ||
getState | ||
}) => { | ||
const { | ||
config | ||
} = getState(); | ||
|
||
if (observer.config !== config) { | ||
observer.config = config; | ||
observer.mutationObserver?.disconnect(); | ||
observer.mutationObserver = new MutationObserver(() => updateTitle(getState)); | ||
observer.mutationObserver.observe( | ||
document.querySelector('title'), | ||
{ subtree: true, childList: true, attributes: true, characterData: true } | ||
); | ||
} | ||
|
||
updateTitle(getState); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaces There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This solution should be more robust as it can react to both the DOM changing and the store changing in ways that are not dependant on exact execution timing (the additional wait broke the test/behavior) |
||
} | ||
}; | ||
|
||
export default observer; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,29 +4,34 @@ import { | |
difference, | ||
filter, | ||
flatten, | ||
forEach, | ||
groupBy, | ||
includes, | ||
intersection, | ||
isEmpty, | ||
isNil, | ||
map, | ||
mergeLeft, | ||
mergeWith, | ||
pluck, | ||
reduce, | ||
values | ||
} from 'ramda'; | ||
|
||
import { IStoreState } from '../store'; | ||
|
||
import { | ||
aggregateCallbacks, | ||
removeRequestedCallbacks, | ||
removePrioritizedCallbacks, | ||
removeExecutingCallbacks, | ||
removeWatchedCallbacks, | ||
addRequestedCallbacks, | ||
addPrioritizedCallbacks, | ||
addExecutingCallbacks, | ||
addWatchedCallbacks, | ||
removeBlockedCallbacks, | ||
addBlockedCallbacks | ||
addBlockedCallbacks, | ||
addRequestedCallbacks, | ||
removeRequestedCallbacks | ||
} from '../actions/callbacks'; | ||
|
||
import { isMultiValued } from '../actions/dependencies'; | ||
|
@@ -45,17 +50,23 @@ import { | |
IBlockedCallback | ||
} from '../types/callbacks'; | ||
|
||
import wait from './../utils/wait'; | ||
|
||
import { getPendingCallbacks } from '../utils/callbacks'; | ||
import { IStoreObserverDefinition } from '../StoreObserver'; | ||
|
||
const observer: IStoreObserverDefinition<IStoreState> = { | ||
observer: ({ | ||
observer: async ({ | ||
dispatch, | ||
getState | ||
}) => { | ||
await wait(0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding this little wait apparently has wide-ranging consequences on the timing of various features. So far:
|
||
|
||
const { callbacks, callbacks: { prioritized, blocked, executing, watched, stored }, paths } = getState(); | ||
let { callbacks: { requested } } = getState(); | ||
|
||
const initialRequested = requested.slice(0); | ||
|
||
const pendingCallbacks = getPendingCallbacks(callbacks); | ||
|
||
/* | ||
|
@@ -78,17 +89,37 @@ const observer: IStoreObserverDefinition<IStoreState> = { | |
1. Remove duplicated `requested` callbacks - give precedence to newer callbacks over older ones | ||
*/ | ||
|
||
/* | ||
Extract all but the first callback from each IOS-key group | ||
these callbacks are duplicates. | ||
*/ | ||
const rDuplicates = flatten(map( | ||
group => group.slice(0, -1), | ||
values( | ||
groupBy<ICallback>( | ||
getUniqueIdentifier, | ||
requested | ||
) | ||
let rDuplicates: ICallback[] = []; | ||
let rMergedDuplicates: ICallback[] = []; | ||
|
||
forEach(group => { | ||
if (group.length === 1) { | ||
// keep callback if its the only one of its kind | ||
rMergedDuplicates.push(group[0]); | ||
} else { | ||
const initial = group.find(cb => cb.initialCall); | ||
if (initial) { | ||
// drop the initial callback if it's not alone | ||
rDuplicates.push(initial); | ||
} | ||
|
||
const groupWithoutInitial = group.filter(cb => cb !== initial); | ||
if (groupWithoutInitial.length === 1) { | ||
// if there's only one callback beside the initial one, keep that callback | ||
rMergedDuplicates.push(groupWithoutInitial[0]); | ||
} else { | ||
// otherwise merge all remaining callbacks together | ||
rDuplicates = concat(rDuplicates, groupWithoutInitial); | ||
rMergedDuplicates.push(mergeLeft({ | ||
changedPropIds: reduce(mergeWith(Math.max), {}, pluck('changedPropIds', groupWithoutInitial)), | ||
executionGroup: filter(exg => !!exg, pluck('executionGroup', groupWithoutInitial)).slice(-1)[0] | ||
}, groupWithoutInitial.slice(-1)[0]) as ICallback); | ||
} | ||
} | ||
}, values( | ||
groupBy<ICallback>( | ||
getUniqueIdentifier, | ||
requested | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Merge callbacks, keep correct changePropIds DIRECT/INDIRECT and last know executionGroup if there is one. Always drop the initial callback if there is one and there are duplicates. |
||
) | ||
)); | ||
|
||
|
@@ -97,7 +128,7 @@ const observer: IStoreObserverDefinition<IStoreState> = { | |
Clean up the `requested` list - during the dispatch phase, | ||
duplicates will be removed for real | ||
*/ | ||
requested = difference(requested, rDuplicates); | ||
requested = rMergedDuplicates; | ||
|
||
/* | ||
2. Remove duplicated `prioritized`, `executing` and `watching` callbacks | ||
|
@@ -312,16 +343,24 @@ const observer: IStoreObserverDefinition<IStoreState> = { | |
dropped | ||
); | ||
|
||
requested = difference( | ||
requested, | ||
readyCallbacks | ||
); | ||
|
||
const added = difference(requested, initialRequested); | ||
const removed = difference(initialRequested, requested); | ||
|
||
dispatch(aggregateCallbacks([ | ||
// Clean up requested callbacks | ||
added.length ? addRequestedCallbacks(added) : null, | ||
removed.length ? removeRequestedCallbacks(removed) : null, | ||
// Clean up duplicated callbacks | ||
rDuplicates.length ? removeRequestedCallbacks(rDuplicates) : null, | ||
pDuplicates.length ? removePrioritizedCallbacks(pDuplicates) : null, | ||
bDuplicates.length ? removeBlockedCallbacks(bDuplicates) : null, | ||
eDuplicates.length ? removeExecutingCallbacks(eDuplicates) : null, | ||
wDuplicates.length ? removeWatchedCallbacks(wDuplicates) : null, | ||
// Prune callbacks | ||
rRemoved.length ? removeRequestedCallbacks(rRemoved) : null, | ||
rAdded.length ? addRequestedCallbacks(rAdded) : null, | ||
pRemoved.length ? removePrioritizedCallbacks(pRemoved) : null, | ||
pAdded.length ? addPrioritizedCallbacks(pAdded) : null, | ||
bRemoved.length ? removeBlockedCallbacks(bRemoved) : null, | ||
|
@@ -330,15 +369,7 @@ const observer: IStoreObserverDefinition<IStoreState> = { | |
eAdded.length ? addExecutingCallbacks(eAdded) : null, | ||
wRemoved.length ? removeWatchedCallbacks(wRemoved) : null, | ||
wAdded.length ? addWatchedCallbacks(wAdded) : null, | ||
// Prune circular callbacks | ||
rCirculars.length ? removeRequestedCallbacks(rCirculars) : null, | ||
// Prune circular assumptions | ||
oldBlocked.length ? removeRequestedCallbacks(oldBlocked) : null, | ||
newBlocked.length ? addRequestedCallbacks(newBlocked) : null, | ||
// Drop non-triggered initial callbacks | ||
dropped.length ? removeRequestedCallbacks(dropped) : null, | ||
// Promote callbacks | ||
readyCallbacks.length ? removeRequestedCallbacks(readyCallbacks) : null, | ||
readyCallbacks.length ? addPrioritizedCallbacks(readyCallbacks) : null | ||
])); | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
export default async (duration: number) => { | ||
let _resolve: any; | ||
const p = new Promise(resolve => _resolve = resolve); | ||
|
||
setTimeout(_resolve, duration); | ||
|
||
return p; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really dislike this artificial delay being required to counteract the additional processing delay in https://github.com/plotly/dash/pull/1385/files#diff-178b2bd05a773877fd1b117eef8b1e8cR63 but I can't think of a better alternative atm.