TiddlyWiki / TiddlyWiki5

A self-contained JavaScript wiki for the browser, Node.js, AWS Lambda etc.
https://tiddlywiki.com/
Other
7.98k stars 1.18k forks source link

[BUG] All server tiddlers are loaded without being requested #4598

Closed danielo515 closed 8 months ago

danielo515 commented 4 years ago

Describe the bug I think that a bug was introduced as part of #4373. For the concrete line of where this has been introduced please look at my comment here: https://github.com/Jermolene/TiddlyWiki5/commit/b95723a022669371cd139b332cf3a64bc438b73e#r38739367

What happens is that, once the server returns the list of available skinny tiddlers then every single tiddler is enqueued to be loaded, and right after this process finishes the job to sync to the server is triggered. This completely defeats the purpose of having a "slim" version of the tiddlers and load them in demand. Prior to this change you could have thousands of tiddlers on the backend and that will not impact on the browser, only what is needed whas loaded, but now this is putting a lot of pressure on the sync mechanism (each tiddler is being loaded individually and in sequence) and on the browser memory.

To Reproduce Steps to reproduce the behavior:

  1. Go to any wiki that is supposed to lazy load tiddlers
  2. Add a bunch of tiddlers, open the console and run $tw.wiki.getTiddler("title").fields. The text field is, as expected, part of the fields. Wait the server to sync changes.
  3. Reload the page, do not open any of the tiddlers previously created and run this on the browser console $tw.wiki.getTiddler("title").fields.
  4. The text field should not be part of it, but it is there

Expected behavior Skinny tiddlers/lazy loaded tiddlers should only be loaded when it's text field is requested and not before.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

Jermolene commented 4 years ago

Hi @danielo515

Go to any wiki that is supposed to lazy load tiddlers

It would be clearer if you could describe the wiki setup more precisely, ideally with reference to the TW5 repo (e.g. "change tiddlywiki.info of editions/tw5.com to look like this:")

Arlen22 commented 4 years ago

Does this problem show up if you lazy load tw5.com?

Arlen22 commented 4 years ago

This command seems to work properly:

tiddlywiki editions/tw5.com-server --listen root-tiddler=$:/core/save/lazy-all
pmario commented 4 years ago

@Jermolene ... I think danielo talks about problems with noteself and the new syncer behaviour.

@danielo515 ... If that's the case, you could use the "old" syncer, if the new one causes too many problems

danielo515 commented 4 years ago

Yes, I'm talking about the new syncer behavior. It is not causing me problems directly, but I think it is an unexpected behavior and unwanted for all the cases that I can think of. It is not that it's not working, it's the behavior which is just incorrect.

danielo515 commented 4 years ago

This command seems to work properly:

tiddlywiki editions/tw5.com-server --listen root-tiddler=$:/core/save/lazy-all

I'll check how that behaves, thanks for the example

Jermolene commented 4 years ago

Thanks @danielo515 I think I've got it now. The skinny tiddler implementation in v5.1.22 has changed: the tiddlyweb syncadaptor now expects that every skinny tiddler will be baked into the TiddlyWiki HTML file with the _is_skinny field indicating that the text field is missing (previously, the absence of the text field indicated a skinny tiddler, which gave us lots of problems because it was ambiguous).

I'm guessing that that isn't possible with NoteSelf because you load tiddlers dynamically after startup? Do you have a loader that runs via rawmarkup before TiddlyWiki boots?

danielo515 commented 4 years ago

I'm guessing that that isn't possible with NoteSelf because you load tiddlers dynamically after startup? Do you have a loader that runs via rawmarkup before TiddlyWiki boots?

Yes, I do, but it is only for a limited amount of default tiddlers. This is an important breaking change regarding sync adaptors.

Not every sync adaptor has the opportunity to inject tiddlers on the HTML file, this seems to be only taking in account the specific needs of tiddlywiki web server and nothing more. Even if I have the oportunity to inject them, that will significantly slow down the startup time! Now I have to query the database for every single tiddler, and iterate them just to include that flag. I really have the impression that sync adaptors are a second class citizen in tiddlywiki plugin ecosystem and that the only one taken in account is the default web server and the blessed flow it defines. It is a daunting task to maintain a sync adaptor to work with tiddlywiki, specially because there is no versioning indication of breaking changes (and there have been many since 5.1.0). Every time I want to spend a weekend adding a new feature I just spend 1/2 weekend debugging tiddlywiki code just to found that a breaking change was introduced two random versions before, and I usually leave the feature implementation for the future because the frustration.

Isn't there a better way to flag a tiddler as skinny? What if the sync adaptor returns that flag when a tiddler is requested instead of baking it into the HTML? Have you weighted any other alternative?

Arlen22 commented 4 years ago

Isn't there a better way to flag a tiddler as skinny? What if the sync adaptor returns that flag when a tiddler is requested instead of baking it into the HTML? Have you weighted any other alternative?

In looking at the code that changed, I'm not quite sure if this is the problem. Previously you load in a massive amount of tiddlers as skinny tiddlers without the text field. Now you just have to iterate through and add a field before inserting it into the wiki. The actual change was in wiki.js, not in the syncer or tiddlyweb adapter, so to me this doesn't seem specific to either.

I'm still not convinced that this is the problem, since the change should mean that lazy loading would not be triggered at all instead of triggered all at once.

Arlen22 commented 4 years ago

The syncer does not refer to _is_skinny anywhere. There is only one place and that is in wiki.js that it is referred to in Javascript.

Arlen22 commented 4 years ago

@Jermolene I think this line is the culprit, but I'm not sure yet: https://github.com/Jermolene/TiddlyWiki5/blob/master/core/modules/syncer.js#L355

Arlen22 commented 4 years ago

@Jermolene How do we load new tiddlers without loading them fat? I think this is the problem. All tiddlers that appear for the first time in the skinny list will get lazy loaded because the current revision is null. I think that if the tiddler is appearing for the first time, it should stay skinny until it is requested, which means the sync adapter would need to add the _is_skinny field to it. Or we could have the server add the _is_skinny field and the sync adapter would detect that and not load the whole tiddler. I'm not sure which way is better.

Arlen22 commented 4 years ago

I really have the impression that sync adaptors are a second class citizen in tiddlywiki plugin ecosystem and that the only one taken in account is the default web server and the blessed flow it defines.

I have felt that way too, but I also think Jeremy may be the lone champion of the server-side stuff. The server-side code is well organized and quite thorough, yet very few people seem to really understand its full potential. I'm not sure why that is. Most people focus on single file wikis as opposed to individual tiddler storage methods like data folders. I guess TiddlyWiki tends to attract users faster than programmers.

dubiouscript commented 4 years ago

I guess TiddlyWiki tends to attract users faster than programmers.

wrt professionally technical / programmers

as i happen to have read this(quote) recently ... and finaly played abit with the node svr

https://lesser.occult.institute/an-opinionated-approach-to-tiddlywiki

It was correctly observed that first-time user onboarding is... rough. It's easy for even professionally technical people to bounce off, having made configuration choices that don't match their goals, and to conclude that TW does not meet their needs.

honestly idk what catogory of technical Literacy i might/should be included in

but ftr i hacked up a (bash httpd)server config https://github.com/Jermolene/TiddlyWiki5/issues/3946#issuecomment-501890267

for tiddly wiki but the sync adapter stuff , im still scratching my head , for the moment =s

qiushihe commented 2 years ago

FWIW I made a PR to allow skinny tiddlers to be only fetched on-demand: https://github.com/Jermolene/TiddlyWiki5/pull/6372

p.s. Please see the section of my PR's description about the order of operation between setting the new flag and responding to the getStatus call in a syncadaptor module.

linonetwo commented 8 months ago

I believe this is fixed, because I use lazy-loading in 5.3.3 every day, with 3 sync adaptors

and they all work fine.

Jermolene commented 8 months ago

Thanks @linonetwo I think this is no longer relevant. The change to the handling of skinny tiddlers was not backwards compatible, but it was necessary to resolve long standing problems.