-
Notifications
You must be signed in to change notification settings - Fork 52
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
Repurpose the datastores to merge values, not replace #1465
Conversation
Hi @joepavitt, |
Still a lot more I can clean out I think, was just taking a snapshot to share progress. |
Does this change the contents of the 'last' message replay, that is received when a page connects or refreshes? |
@colinl,
So you have 1 last msg per ui node. That last msg is continiously updated by merging new input messages into it (while ignoring all ui_update stuff). Which means it is in fact not correct anymore to talk about the last msg, because it is in fact build by merging N successive input messages.
As a result you can replay that (last) merged message, because it always contains the (last) complete state. Although I am in a very negative mood at the moment, I can't make up any scenario at the moment that would fail with this new setup ;-) It looks like a decent solution to me. |
OK, so potentially this could break existing contributed nodes. I think I will probably be ok because that is basically what I do myself anyway. |
Not sure if it will break existing nodes. Like you say you (and I) already have added some code to workaround the data loss. I assume there will be some obsolete code in the third-party ui nodes. |
Yes, I should be able to simplify the code. |
Will this affect ui-template node operation? |
No, anything already built will work as-is, this will mostly be cleaning the underlying core nodes. |
# Conflicts: # ui/src/widgets/ui-number-input/UINumberInput.vue # ui/src/widgets/ui-slider/UISlider.vue # ui/src/widgets/ui-text-input/UITextInput.vue # ui/src/widgets/ui-text/UIText.vue
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.
LGTM. Will likely break #1237 again sigh
Description
msg
in thedatastore
, wemerge
the objects, overriding new values where relevant, but persistent values from previousmsg
's that have not had new values set.value
being available in the centralisedvuex
store.Related Issue(s)
Closes #1294