Add server sent events #5279

NicolasPetton · 2020-12-14T12:56:50Z

This is a follow-up to the original PR by @Arlen22 here: #4609.

Original PR description

This is related to Issue #4608 and also fixes Issue #4605.

To test this, run node tiddlywiki.js editions/server --listen then open two pages side-by-side and watch changes propagate in almost real time.

pmario · 2020-12-28T13:37:26Z

@Jermolene ... bump

NicolasPetton · 2021-01-05T19:02:48Z

@Jermolene are there any changes you'd like me to do?

NicolasPetton · 2021-01-13T10:29:36Z

@Jermolene I just rebased the branch.

I can also rerwite the history if that's too many commits (I kept the commits from @Arlen22 but I can squash some of them to make the history cleaner).

Jermolene

Hi @NicolasPetton apologies for the delay. Although I've been merging new stuff as it pops up, I haven't yet had a chance to methodically go back through the tickets we identified in #5231.

There's a couple of minor coding style comments. I'm also wondering if we need a configuration setting to turn off the SSE server; it could conceivably be a vector for DDOS attacks. We also need documentation updates.

Finally, I'm wondering if it's worth merging this without updating the syncer/syncadaptor to take advantage of it.

core/modules/server/server-sent-events.js

plugins/tiddlywiki/tiddlyweb/sse-server.js

NicolasPetton · 2021-01-13T14:30:44Z

There's a couple of minor coding style comments.

I'll fix them.

I'm also wondering if we need a configuration setting to turn off the SSE server; it could conceivably be a vector for DDOS attacks.

Would you accept that improvement in a separate PR?

We also need documentation updates.

Would you like documentation updates as part of this PR? If so, could you point out which tiddlers need updating?

Finally, I'm wondering if it's worth merging this without updating the syncer/syncadaptor to take advantage of it.

Sorry, I'm not following. The PR adds a route /events/plugins/tiddlywiki/tiddlyweb for tiddlyweb SSE in sse-server.js. Have I missed something else?

pmario · 2021-01-13T16:32:13Z

@Jermolene from the OP

To test this, run node tiddlywiki.js editions/server --listen then open two pages side-by-side and watch changes propagate in almost real time.

I think it does take advantage of the new funtionality

pmario · 2021-01-13T16:33:58Z

I think the configuration may be a new parameter to the --listen command see: https://tiddlywiki.com/#ListenCommand

For compatibility and security reasons, it may be switched OFF by default. So if the new parameter is present it will be switched on

Jermolene · 2021-01-13T17:28:39Z

I'll fix them.

Thanks.

Would you accept that improvement in a separate PR?

Would you like documentation updates as part of this PR? If so, could you point out which tiddlers need updating?

It's generally best to include the docs in the same PR but we do fairly often merge them separately.

Finally, I'm wondering if it's worth merging this without updating the syncer/syncadaptor to take advantage of it.

Sorry, I'm not following. The PR adds a route /events/plugins/tiddlywiki/tiddlyweb for tiddlyweb SSE in sse-server.js. Have I missed something else?

The integration with the syncer is primitive; the client calls $tw.syncer.syncFromServer() when it receives changes from the server, but we don't do anything to disable the usual polling based syncing.

Jermolene · 2021-01-13T17:30:23Z

I think the configuration may be a new parameter to the --listen command see: https://tiddlywiki.com/#ListenCommand

For compatibility and security reasons, it may be switched OFF by default. So if the new parameter is present it will be switched on

That makes sense. The client will need to be able to run correctly with either setting.

* Extract helper functions for handling wikis and connections. * Replace JSDoc comments. * Fix formatting according to TW core. * Simplify the logic for adding and removing connections.

Fix formatting according to TW core.

Fix formatting and comments following TW core guidelines.

startsWith is part of ES2015, while TiddlyWiki uses the 5.1 dialect.

* If not set to "yes", disabled SSE request handling. * Add documentation for the parameter in core/language/en-GB/Help/listen.tid * Add new tiddler editions/tw5.com/tiddlers/webserver/WebServer Parameter_ sse-enabled.tid

* Add sse_enabled to /status JSON response * Store syncer polling status in $:/config/SyncDisablePolling * Handled disabling polling in core/modules/syncer.js

NicolasPetton · 2021-01-13T21:02:45Z

@Jermolene I've fixed all the styling issues, plus:

Added a new WebServer parameter sse-enabled, which is disabled by default.
When enabled, polling is disabled in syncer.js. The /status JSON response now contains a new sse_enabled field.
I documented the new sse-enabled param in a new tiddler and added a list item in core/language/en-GB/Help/listen.tid

pmario

The code looks good to me

Jermolene · 2021-01-15T10:37:45Z

Bravo, and thank you @NicolasPetton looks great.

linonetwo · 2021-06-03T14:45:26Z

Wow, amazing feature, will this make #3060 possible?

Jermolene · 2021-06-03T18:08:23Z

Wow, amazing feature, will this make #3060 possible?

Sadly no: #3060 is about syncing between the serverside wiki and the file system, while SSE just improve things between the clientside wiki and the serverside wiki.

Jermolene · 2021-07-14T14:15:36Z

As discussed in #5853 (comment), I'm reverting this PR in favour of a slightly different approach: we will add the hooks required to enable SSE implementations to be undertaken in plugins.

Jermolene · 2021-07-14T14:16:37Z

Apologies, to clarify that the reverting is being done in #5853

This reverts commit 17b4f53 according to Github Desktop. git checkout that commit revert commit in GitHub Desktop git switch -c revert-sse uncommit in Github Desktop switch to master, bringing changes resolve deletions with command line

pmario · 2021-07-14T15:45:23Z

Wow, amazing feature, will this make #3060 possible?

@linonetwo ... You can user curl or an other tool to send new files to the server instead of creating them with an editor. So if the serve gets the info about new tiddlers that way all the syncing should be done automatically. See the discussion on GG

This reverts commit 17b4f53 according to Github Desktop. git checkout that commit revert commit in GitHub Desktop git switch -c revert-sse uncommit in Github Desktop switch to master, bringing changes resolve deletions with command line

This was referenced Dec 14, 2020

Add Server Sent Events #4609

Closed

Use server-sent events for notifying changes to browser #1581

Closed

NicolasPetton force-pushed the add-server-sent-events branch from 5e2be7d to 5b90300 Compare December 14, 2020 13:11

saqimtiaz mentioned this pull request Dec 14, 2020

Tickets to be considered for post-v5.1.23 #5231

Closed

NicolasPetton force-pushed the add-server-sent-events branch from 5b90300 to 4573d8c Compare December 14, 2020 13:15

NicolasPetton force-pushed the add-server-sent-events branch from 4573d8c to ffa2141 Compare December 31, 2020 09:25

NicolasPetton force-pushed the add-server-sent-events branch 2 times, most recently from 469dbef to 979b3ee Compare January 13, 2021 10:28

Jermolene reviewed Jan 13, 2021

View reviewed changes

core/modules/server/server-sent-events.js Outdated Show resolved Hide resolved

core/modules/server/server-sent-events.js Outdated Show resolved Hide resolved

plugins/tiddlywiki/tiddlyweb/sse-server.js Outdated Show resolved Hide resolved

NicolasPetton force-pushed the add-server-sent-events branch 2 times, most recently from 41ba026 to 1e70cfb Compare January 13, 2021 19:32

Arlen22 added 11 commits January 13, 2021 21:59

Create server-sent-events.js

965ccad

Create sse-change-listener.js

145af39

Implement server sent events

a0fb5cd

Convert to ES5 and wrap in function

eca1147

Use the host string from tiddlyweb

0ed90d2

Improve comments in sse-server.js

267244d

Can't use object reference as key

0b0baa6

Add retry timeout

f4adda0

Fix a bug

ffaaca7

bug fix

534816d

Fix formatting

4f85db0

Arlen22 and others added 10 commits January 13, 2021 21:59

Fix ES5 compat

2d11e9d

capitalize comments

244a1ab

more fixes

66d780f

Refactor tiddlywek/sse-server.js

8543941

* Extract helper functions for handling wikis and connections. * Replace JSDoc comments. * Fix formatting according to TW core. * Simplify the logic for adding and removing connections.

Fix formatting of tiddlyweb/sse-client.js

aee5cb4

Fix formatting according to TW core.

Fix formatting of server-sent-events.js

2c1337b

Fix formatting and comments following TW core guidelines.

Extract a debounce function in sse-client.js

4ffd0d7

Avoid using startsWith in server-sent-events.js

c1d8283

startsWith is part of ES2015, while TiddlyWiki uses the 5.1 dialect.

New sse-enabled WebServer parameter

b380a73

* If not set to "yes", disabled SSE request handling. * Add documentation for the parameter in core/language/en-GB/Help/listen.tid * Add new tiddler editions/tw5.com/tiddlers/webserver/WebServer Parameter_ sse-enabled.tid

Disable polling for changes if SSE is enabled

5a9ae03

* Add sse_enabled to /status JSON response * Store syncer polling status in $:/config/SyncDisablePolling * Handled disabling polling in core/modules/syncer.js

NicolasPetton force-pushed the add-server-sent-events branch from 1e70cfb to 5a9ae03 Compare January 13, 2021 20:59

NicolasPetton requested a review from Jermolene January 13, 2021 21:02

NicolasPetton added 2 commits January 14, 2021 09:20

Simply boolean logic in syncer.js

c6a5252

Delete trailing whitespaces in syncer.js

c2be5b8

pmario reviewed Jan 14, 2021

View reviewed changes

Jermolene merged commit 17b4f53 into TiddlyWiki:master Jan 15, 2021

This was referenced Jan 15, 2021

[IDEA] Is now the time to add server sent events to core? #4608

Closed

Fix a typo in WebServer Parameter_ sse-enabled.tid #5407

Merged

saqimtiaz mentioned this pull request May 23, 2021

TW 5.2.0 release imminent - last call for bug fixes and outstanding PRs #5717

Closed

Jermolene mentioned this pull request Jul 14, 2021

Improvements to SSE #5853

Closed

Arlen22 mentioned this pull request Jul 16, 2021

Add syncer status hook and option to disable polling #5882

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add server sent events #5279

Add server sent events #5279

NicolasPetton commented Dec 14, 2020

pmario commented Dec 28, 2020

NicolasPetton commented Jan 5, 2021

NicolasPetton commented Jan 13, 2021

Jermolene left a comment

NicolasPetton commented Jan 13, 2021

pmario commented Jan 13, 2021

pmario commented Jan 13, 2021 •

edited

Loading

Jermolene commented Jan 13, 2021

Jermolene commented Jan 13, 2021

NicolasPetton commented Jan 13, 2021

pmario left a comment

Jermolene commented Jan 15, 2021

linonetwo commented Jun 3, 2021

Jermolene commented Jun 3, 2021

Jermolene commented Jul 14, 2021

Jermolene commented Jul 14, 2021

pmario commented Jul 14, 2021

Add server sent events #5279

Add server sent events #5279

Conversation

NicolasPetton commented Dec 14, 2020

Original PR description

pmario commented Dec 28, 2020

NicolasPetton commented Jan 5, 2021

NicolasPetton commented Jan 13, 2021

Jermolene left a comment

Choose a reason for hiding this comment

NicolasPetton commented Jan 13, 2021

pmario commented Jan 13, 2021

pmario commented Jan 13, 2021 • edited Loading

Jermolene commented Jan 13, 2021

Jermolene commented Jan 13, 2021

NicolasPetton commented Jan 13, 2021

pmario left a comment

Choose a reason for hiding this comment

Jermolene commented Jan 15, 2021

linonetwo commented Jun 3, 2021

Jermolene commented Jun 3, 2021

Jermolene commented Jul 14, 2021

Jermolene commented Jul 14, 2021

pmario commented Jul 14, 2021

pmario commented Jan 13, 2021 •

edited

Loading