Title and Icon are not resetting properly after browser refresh and null setting

[gayanSandamal]

Current Behavior

Title and icon values are not resetting back to the default after setting the particular dynamic property values to null

Expected Behavior

Widgets should display the original value after settings dynamic properties to null

Steps To Reproduce

1. Add a text input or gauge node.
2. Set a title via widget properties.
3. Update title or icon dynamically using an inject node.
4. Reload the dashboard 2.0 after the particular value got updated in the UI.
5. set null to dynamic properties and inject.

Environment

• Dashboard version: 1.15.0
• Node-RED version: 4.0.2
• Node.js version: 20
• npm version:
• Platform/OS:
• Browser:

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

[bartbutenaers]

Hi @gayanSandamal,

I think this is caused by an unimplemented TODO comment that you can find in the datatracker file. Let's for example have a look at the UIText widget:

So instead of implementing the onInput function in each widget again (which will contain mostly the same logic for lots of widgets), a lot of widgets (re)use the onInput function which has been implemented in the datatracker. You can see that because null is passed to the datatracker, instead of passing a custom onInput function.

Suppose you inject an input msg whose payload contains the text:

1. The input message arrives on the frontend
2. Since there is no custom onInput specified, you arrive in the default onInput code which stores ALL input messages into the datastore.
3. Those messages are mapped to the widget
4. So the computed field value gets that latest stored message. Since the last msg has a payload, that payload value (containing the text) will be returned.
5. That way your text from the payload will be binded and displayed in your html.

So far so good.

But afterwards you inject a message without payload, just to set some dynamic property. All 5 steps are repeated. However in step 4 there is no payload so an empty string is returned. That will be binded to your html, and as a result your title disappears.

So I think the datatracker should be fixed. Until recently it was ok that the datatracker stored ALL messages into the datastore. But since the mechanism of dynamic properties has been introduced, only (messages with) payloads should be stored. Because we only use payloads from the input messages via the datastore. All dynamic properties from those messages are stored in the statestore. Allthough I am not 100% sure that only payloads are being used. Perhaps msg.options and (legacy) stuff like that are still stored in messages in datastore. Not sure, you need to check that!!!

Therefore I 'think' the default onInput code in the datatrackers should only store messages that have a payload:

            socket.on('msg-input:' + widgetId, (msg) => {
                // check for common dynamic properties cross all widget types
                checkDynamicProperties(msg)

                if (onInput) {
                    // sometimes we need to have different behaviour
                    onInput(msg)
                } else {
                    // but most of the time, we just care about the value of msg
                    if (typeof msg.payload !== 'undefined') {
                        store.commit('data/bind', {
                            widgetId,
                            msg // TODO: we should sanitise what is stored in the store?
                            // One way to do this is to permit only keys explicitly listed in the widget's config (default to topic+payload if none are specified)
                            // A smarter? way to do this is to scan the template for msg.? binds and store only those keys
                            // For now, we'll just store the whole msg
                        })
                    }
                }
            })

Imho it is very confusing to store entire messages, if you only need their payload. I would say it is better to store the payload (which contains the data) in the datastore...

Hopefully it makes sense what I am writing. Bart

[joepavitt]

Imho it is very confusing to store entire messages, if you only need their payload. I would say it is better to store the payload (which contains the data) in the datastore...

I agree, there is nothing to warrant third party nodes saving just payloads.

Core widgets are implemented to store msg, but we can correct this in the future. We are have far more pressing issues elsewhere though, which will have more obvious impact to users, whereas this, we can invest 2 weeks of dev time to update all core nodes, and it'll have zero impact on the users interactions with Dashboard

[bartbutenaers]

@joepavitt Yes I absolutely agree that you currently have more important things to do. And enjoy your holidays...

[gayanSandamal]

Hi @gayanSandamal,

I think this is caused by an unimplemented TODO comment that you can find in the datatracker file. Let's for example have a look at the UIText widget:

So instead of implementing the onInput function in each widget again (which will contain mostly the same logic for lots of widgets), a lot of widgets (re)use the onInput function which has been implemented in the datatracker. You can see that because null is passed to the datatracker, instead of passing a custom onInput function.

Suppose you inject an input msg whose payload contains the text:

1. The input message arrives on the frontend
2. Since there is no custom onInput specified, you arrive in the default onInput code which stores ALL input messages into the datastore.
3. Those messages are mapped to the widget
4. So the computed field value gets that latest stored message. Since the last msg has a payload, that payload value (containing the text) will be returned.
5. That way your text from the payload will be binded and displayed in your html.

So far so good.

But afterwards you inject a message without payload, just to set some dynamic property. All 5 steps are repeated. However in step 4 there is no payload so an empty string is returned. That will be binded to your html, and as a result your title disappears.

So I think the datatracker should be fixed. Until recently it was ok that the datatracker stored ALL messages into the datastore. But since the mechanism of dynamic properties has been introduced, only (messages with) payloads should be stored. Because we only use payloads from the input messages via the datastore. All dynamic properties from those messages are stored in the statestore. Allthough I am not 100% sure that only payloads are being used. Perhaps msg.options and (legacy) stuff like that are still stored in messages in datastore. Not sure, you need to check that!!!

Therefore I 'think' the default onInput code in the datatrackers should only store messages that have a payload:

            socket.on('msg-input:' + widgetId, (msg) => {
                // check for common dynamic properties cross all widget types
                checkDynamicProperties(msg)

                if (onInput) {
                    // sometimes we need to have different behaviour
                    onInput(msg)
                } else {
                    // but most of the time, we just care about the value of msg
                    if (typeof msg.payload !== 'undefined') {
                        store.commit('data/bind', {
                            widgetId,
                            msg // TODO: we should sanitise what is stored in the store?
                            // One way to do this is to permit only keys explicitly listed in the widget's config (default to topic+payload if none are specified)
                            // A smarter? way to do this is to scan the template for msg.? binds and store only those keys
                            // For now, we'll just store the whole msg
                        })
                    }
                }
            })

Imho it is very confusing to store entire messages, if you only need their payload. I would say it is better to store the payload (which contains the data) in the datastore...

Hopefully it makes sense what I am writing. Bart

Hi @bartbutenaers ,

Thanks so much for digging into this and providing such a detailed explanation! I really appreciate the time and effort you put into breaking it all down. Your insights are super helpful. We’ll dive into this with the suggestions you made once we started working on this particular issue and see how we can move forward with this.

[Steve-Mcl]

I spent some time looking into this today and i think it is far simpler.

When emitConfig is called we currently merge any dynamic updates into props this compounds changes when the page is refreshed.

Since we have a design pattern now (with updateDynamicProperty and getProperty) the dynamic overrides are applied client side at render so there really is no need to compound changes.

By simply chaning the line of code from widget.props = { ...widget.props, ...state } to widget.props = { ...widget.props } the real (original) props are sent on config emit and the design patter for dynamic updates are applied as usual

Demo: BEFORE

Demo: AFTER

[bartbutenaers]

Hi @Steve-Mcl , Thanks for digging into this further!! Is this somehow related to mapState('data', ['messages', 'properties'])? Not really sure what kind of magic it all does unfortunately, so a bit difficult to give decent feedback on your comments. I have already an open issue issue (first task) on my new video player node about the use of mapData.