[BUG] Reuse of $:/core/ui/EditTemplate/fields broken in v5.2.3

[Jermolene]

Describe the bug

The changes in #6511 have resulted in significant data loss for at least one user:

https://talk.tiddlywiki.org/t/is-that-a-bug-undefined-variable-on-the-core-ui-edittemplate-fields-cause-all-the-tiddlers-to-be-deleted-when-creating-a-new-field/5247

The problem is that the changes in #6628 are not backwards compatible. They require that additional variables are set before transcluding $:/core/ui/EditTemplate/fields

Expected behavior

No response

To Reproduce

No response

Screenshots

No response

TiddlyWiki Configuration

v5.2.3

Additional context

No response

[Jermolene]

Do you have any thoughts about this @FlashSystems?

[FlashSystems]

I've had a look at the code and found the problem: The clean-up-routine that deletes all the temporary tiddlers contains a prefix-filter to clean up all temporary tiddlers that were created for different types of input fields. These share a common prefix to make them easy to find. This prefix-filter uses the newFieldValueTiddlerPrefix variable. If this variable is empty, the prefix-filter matches all tiddlers :fearful:. I did not expect anyone reaching directly into the core and calling this out of context. This was definitely not good defensive programming practice :disappointed: I'll create a PR that creates a safeNewFieldValueTiddlerPrefix that makes sure that, if there is no value set, a dummy value will be created that matches nothing. I'll also limit the filter to tiddlers starting with $:/temp/NewFieldValue to make sure that a non-empty, but still bogus value for newFieldValueTiddlerPrefix can only cause very little damage.

[Jermolene]

I've had a look at the code and found the problem: The clean-up-routine that deletes all the temporary tiddlers contains a prefix-filter to clean up all temporary tiddlers that were created for different types of input fields. These share a common prefix to make them easy to find. This prefix-filter uses the newFieldValueTiddlerPrefix variable. If this variable is empty, the prefix-filter matches all tiddlers 😨. I did not expect anyone reaching directly into the core and calling this out of context. This was definitely not good defensive programming practice 😞 I'll create a PR that creates a safeNewFieldValueTiddlerPrefix that makes sure that, if there is no value set, a dummy value will be created that matches nothing. I'll also limit the filter to tiddlers starting with $:/temp/NewFieldValue to make sure that a non-empty, but still bogus value for newFieldValueTiddlerPrefix can only cause very little damage.

Great, thank you @FlashSystems

[FlashSystems]

Hello @Jermolene, while fixing the data loss bug, I discovered another problem on my way: The input field for the value is missing, if the $:/core/ui/EditTemplate/fields tiddler is transcluded directly.

This is because the get-field-value-tiddler-filter is defined within EditTemplate.tid and fields.tid uses this definition. I've done it this way to prevent code duplication and because the macro uses variables (like newFieldValueTiddlerPrefix) defined in EditTemplate.tid.

For fixing this problem as well, I'm currently stuck between a rock and a hard place: To make the FieldEditorCascade work, fields.tid needs some common infrastructure that is currently set up by EditTemplate.tid. That's the case for most of the other parts in $:/core/ui/EditTemplate/ (like type.tid for example) as well. I could move some of it into fields.tid to make this use case work again, but this would alter the interface again and risk more regressions in other places. It would be a deviation from how the other parts of the EditTemplate work, and it would make the code much more complex.

It all boils down to one question: What is the supported interface of TiddlyWiki? Is reaching into the core and transcluding a "random" tiddler a supported use case? If that's the case, I see no chance of getting something as complex as the FieldEditorCascade change into TiddlyWiki again without a major version change. Don't get me wrong. Data loss is unacceptable and must be fixed as soon as possible. No discussion about that. But it should be made clear, what the stability guarantees are for Plugin- and PR-Authors. If changing or adding mandatory parameters to undocumented core tiddlers is not an option, changes like the FieldEditorCascade must be delayed for the next major release in the future.

Could you or any of the more experienced TW5 contributors advise on how to handle this situation?

[Jermolene]

It all boils down to one question: What is the supported interface of TiddlyWiki? Is reaching into the core and transcluding a "random" tiddler a supported use case?

We do try to support people transcluding arbitrary core templates, which does mean that the core templates are an implicit API. I don't think it's been a problem before. Perhaps we should have waited for #6666 to land so that we can use parameterised transclusion here; it will make things much easier because it allows default values to be specified for missing parameters.

[Jermolene]

Could you or any of the more experienced TW5 contributors advise on how to handle this situation?

Can we fix it by duplicating some code? Inside the template we could check for the variables being missing, and assign the default values if so.

[FlashSystems]

Hello @Jermolene,

thank you for the fast response and the clearification. I'll take this into account on my next PRs.

Yes, I can solve this by duplicating some code and supplying a default value. I'll put a comment into the tiddler to warn about the duplication. Just for my peace of mind. I really don't like this solution because duplicated code has bitten me more than once while modifying TiddlyWiki. :disappointed: But it seems to be the only way.

I'll give these two fixes some more testing and create a PR ASAP.

[FlashSystems]

Hello @Jermolene,

seems like I forgot the auto-close "fixes" line in the PR. Could you please close this bug as it was fixed by PR #7092. Thank you.

[Jermolene]

Thank you, @FlashSystems