TiddlyWiki / TiddlyWiki5

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

[BUG] setTiddlerData triggers a changed tiddler event even if no change #8784

Open ittayd opened 4 days ago

ittayd commented 4 days ago

Describe the bug

it calls this.addTiddler which in turn calls this.enqueueTiddlerEvent which causes a refresh even if nothing changed.

this can sometimes cause an infinite loop if data is set during a refresh. In particular it happens for the full-text-search plugin as the count of results is refreshed (of course it is bad to set data as part of a filter evaluation)

Expected behavior

the change event should not trigger

To Reproduce

No response

Screenshots

No response

TiddlyWiki Configuration

5.3.5

Additional context

No response

pmario commented 3 days ago

The problem is backwards compatibility. At the moment the code looks like this:

https://github.com/TiddlyWiki/TiddlyWiki5/blob/dabbc1bacc35b3981959065f88adbb3b7ec1c2ff/core/modules/wiki.js#L954-L969

As you found out, .addTiddler is called using new $tw.Tiddler(creationFields,existingTiddler,fields,newFields,modificationFields), which always sets creationFields and modificationFields.

It may be possible to suppress this.addTiddler() if and only if options.suppressTimestamp === true and a temporary tiddler tmpTiddler.isEqual(tiddler, excludeFields) returns true. It needs to be a tiddler, since isEqual() first checks if(!(tiddler instanceof $tw.Tiddler))

See: https://github.com/TiddlyWiki/TiddlyWiki5/blob/dabbc1bacc35b3981959065f88adbb3b7ec1c2ff/boot/boot.js#L1076

Jermolene commented 2 days ago

Hi @ittayd the underlying issue is that the core is written on the assumptions that the tiddler store will not be changed during a refresh cycle. Breaking that assumption will cause oddities and bugs that are unpredictable.

Independently of that fact, there is the question of whether wiki.addTiddler() being called without actually changing any values should invoke a special case and not trigger a refresh cycle. I don't see the necessity for that complexity. Checking whether two tiddlers have the same value is a good order of magnitude more time consuming than simple storing it away.

pmario commented 2 days ago

I also had concerns about the performance implications