Hi Team, @marcus-j-davies, pointed out to me that when we update a node which has a new property added, the default value will not be used: When opening an existing node, the default value of the new property will not be used.
Which counter-intuitively, @knolleary why this choice?
This requires developers to do:
//oneditprepare
if (node.myNewProperty === undefined) {
$("node-input-myNewProperty").val("defaultValue");
}
The value being undefined and undefined not being exportable why not consider that the property is missing and therefore use the default value?
Technically this is a change of the interface contract. Developers may do that and regularly do. Yet they have to ensure that prior versions are "upgraded" accordingly. The non-existence of a property gives a clear signal that there's action necessary; and it might be more to setup than just give a default value to a non-existing property. If you silently fill the gaps, we would need a different mechanism to get that signal.
This is one of those "we didn't think of it at the time" items that was around for long enough that when it was spotted, we couldn't quite decide if it was 'safe' to automatically apply the defaults and decided to err on the side of caution.
We always get complaints when a user opens an edit dialog, makes no changes, clicks 'okay' and then the node is marked as having changed. That is what would happen in this scenario. We don't like causing unexpected behaviours.
There is also the consideration that some nodes might rely on this behaviour to tell if it was handling an 'old' node or not - so it could apply additional logic to help upgrade the node.
The runtime side of a node will always have to deal with a property being undefined - there's no guarantee a user will have opened the editor and deployed any changes since the node was upgraded.
Whilst itāsā¦. Frustrating, is that the right word? Having to fill in a default
I usually categorise this as a breaking change to my nodes, and sacrifice the "polyfill" approach by asking users to provide the value for the new property, if they already have the node deployed
validateNodeProperty allows you to signal that something is wrong.
For the runtime part, I completely agree with its behavior.
For the editor part, whether the developer defines the default value himself or whether it is the node doesn't change much.
This is why I attach big importance to validateNode (input validation) because it shows that a node has a fault and that the user must act. Of course until the user takes action, the node must continue to run normally.
If the user sees that his node has an error he will open the node then check each input then close the node and the error will have disappeared. If no message in the validation (red triangle) indicates that the property could be removed or something else, the user will be unhappy... because he was not warned
The current problem is that each node hangs out old properties because the user does not know that he must redeploy his node.
That's why I believe that the default value can help, of course the developer must also check the migration
I was a little long but I hope you understood my idea
to my mind there is (slight) frustration either way. As-is the user has to go in and set a default. If fixed to auto supply the default then as Nick says the user opens and close the node and gets a changed flag even though nothing appeared to change (though of course there is now a new field they may not have noticed before).
Personally I think a) it means the node developer doesn't have to add his own "new field" handling code (1-3 lines just to set default). and b) the user annoyance is minimal. If a node really needs to do special handling the field can still be set to default value:undefined on the html side.
Of course as this is a small change in behaviour it would be a v4 item.
It's tricky, if I take the example of an addition of functionality, it's not an error. As a deprecated property, as long as we stay on a non-major version, it should not be an error but after updating to the major version it becomes an error (note that the migration must still exist)
It depends on the change, in fact
Yes, that's my recommendation, mark the node in error only if the change is breaking and not progressive like a property addition
If I return to the subject of this discussion:
Let's imagine a new property is added, it is not a breaking change so the node is not in error, as long as the user does not modify his node nothing happens but if he opens it, the new field appears. Using the default value or using the value defined in oneditprepare doesn't change anything. Will the user notice the new field or not, that...
However, if the change is other, it is up to the developer to correctly migrate the value and I agree with you on this point.
I expressed myself badly, but I insist that my request only concerns the case of a new node property, this case where the user does not have to act obligatorily.
If I take your answer, if it's a new property and therefore a new field, the user should notice the change and understand the reason of the modified node.
But I also insist that for ALL other cases, it is up to the developer to manage and inform its users
Does the validation behavior change based on the ārequiredā value?
In the past when I was adding features and extra settings, I had tried to make some migration code to alleviate some burden from the end user. I canāt remember exactly, but it may have been in response to somebody sharing a flow that was based on an older version of the node and had missing properties. It got complicated and on the next update of the node I removed the migration code. I realize now that maybe I could have just marked the field as not being required and just did my normal checking on the back end.
