-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Issue 1350 - Multi-input callback with sync event handling #1385
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
Changes from 5 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; | ||
|
||
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 | ||
---|---|---|---|---|
@@ -1,5 +1,6 @@ | ||||
import { | ||||
all, | ||||
assoc, | ||||
concat, | ||||
difference, | ||||
filter, | ||||
|
@@ -10,6 +11,9 @@ import { | |||
isEmpty, | ||||
isNil, | ||||
map, | ||||
mergeAll, | ||||
partition, | ||||
pluck, | ||||
values | ||||
} from 'ramda'; | ||||
|
||||
|
@@ -45,14 +49,18 @@ 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(); | ||||
|
||||
|
@@ -79,25 +87,35 @@ const observer: IStoreObserverDefinition<IStoreState> = { | |||
*/ | ||||
|
||||
/* | ||||
Extract all but the first callback from each IOS-key group | ||||
these callbacks are duplicates. | ||||
Group callbacks by identifier and partition based on whether there are duplicates or not. | ||||
*/ | ||||
const rDuplicates = flatten(map( | ||||
group => group.slice(0, -1), | ||||
values( | ||||
groupBy<ICallback>( | ||||
getUniqueIdentifier, | ||||
requested | ||||
) | ||||
const [rWithoutDuplicates, rWithDuplicates] = partition(rdg => rdg.length === 1, 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. |
||||
) | ||||
)); | ||||
|
||||
/* | ||||
Flatten all duplicated callbacks into a list for removal | ||||
*/ | ||||
const rDuplicates = flatten(rWithDuplicates); | ||||
|
||||
/* | ||||
Merge duplicate groups into a single callback - merge by giving priority to newer callbacks | ||||
*/ | ||||
const rMergedDuplicates = map(group => assoc( | ||||
'changedPropIds', | ||||
mergeAll(pluck('changedPropIds', group)), | ||||
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. In order to ensure
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. @alexcjohnson Thanks for the heads up. Also distinguishing the initial callback from subsequent callbacks 8b6ff26 |
||||
group[0] | ||||
), rWithDuplicates); | ||||
|
||||
/* | ||||
TODO? | ||||
Clean up the `requested` list - during the dispatch phase, | ||||
duplicates will be removed for real | ||||
*/ | ||||
requested = difference(requested, rDuplicates); | ||||
requested = concat(flatten(rWithoutDuplicates), rMergedDuplicates); | ||||
|
||||
/* | ||||
2. Remove duplicated `prioritized`, `executing` and `watching` callbacks | ||||
|
@@ -319,6 +337,8 @@ const observer: IStoreObserverDefinition<IStoreState> = { | |||
bDuplicates.length ? removeBlockedCallbacks(bDuplicates) : null, | ||||
eDuplicates.length ? removeExecutingCallbacks(eDuplicates) : null, | ||||
wDuplicates.length ? removeWatchedCallbacks(wDuplicates) : null, | ||||
// Add merged-duplicated callbacks | ||||
rMergedDuplicates.length ? addRequestedCallbacks(rMergedDuplicates): null, | ||||
// Prune callbacks | ||||
rRemoved.length ? removeRequestedCallbacks(rRemoved) : null, | ||||
rAdded.length ? addRequestedCallbacks(rAdded) : 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; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -677,7 +677,7 @@ def c2(children): | |
|
||
@app.callback([Output("a", "children")], [Input("c", "children")]) | ||
def c3(children): | ||
return children | ||
return (children,) | ||
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. I assume that this is due to timing changes: The |
||
|
||
dash_duo.start_server(app, **debugging) | ||
|
||
|
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.