-
Notifications
You must be signed in to change notification settings - Fork 91
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
feat: Use notify push for sync messages during editing #4585
Conversation
3f500d8
to
6a3cc72
Compare
1 flaky tests on run #11336 ↗︎
Details:
cypress/e2e/nodes/Links.spec.js • 1 flaky test
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
6a3cc72
to
90c9562
Compare
Thanks a lot for looking into this. I'm really curious to play with it. 😍
Just one remark. Autosave is independent from the polling backend these days. It's triggered by text/src/services/SyncService.js Line 91 in 14d29b1
Never the less I agree that having a fallback for a start might still be worthwhile. |
576e874
to
eed2413
Compare
f4a7d2b
to
69fc3f4
Compare
69fc3f4
to
5a46b70
Compare
5a46b70
to
3570db2
Compare
This is good from my side. I did quite some testing locally with different scenarios of browsers going offline, coming back, overwriting the file externally and it seems to be quite smooth. Nevertheless I added a feature flag to opt-in for now, so we can maybe testrun this a bit more on a daily instance. |
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 did some more reading on the notify_push app and how it's used, and with my basic understanding of the app and websockets, it looks okay to me. I will approve in case it counts here :)
|
Signed-off-by: Julius Härtl <[email protected]>
3570db2
to
efb36a1
Compare
ca024ac
to
f352174
Compare
Signed-off-by: Julius Härtl <[email protected]>
f352174
to
f468541
Compare
Signed-off-by: Julius Härtl [email protected]
📝 Summary
This is a first PoC to make use of notify_push for syncing steps back to other editors instead of regular polling. We still poll every 30 seconds as a fallback and to ensure that the autosave requests are still triggered until #4424 is there (even with that some fallback sync might still be worth).
Can be manually enabled for now with
🚧 TODO
🏁 Checklist
npm run lint
/npm run stylelint
/composer run cs:check
)