form.key inside arrays

[mnzaki]

Bug

form.key inside arrays appends the array path and empty string "" after the actual key, for example giving ["names", 0, "names", ""] for the first array item in an array with key ["names"].

Gist/Plunker/Demo

Plunker demonstrating the issue

@json-schema-form/angular-schema-form-lead

[mnzaki]

And it has more weird behavior when in arrays in objects. Plunker updated to demonstrate.

[Anthropic]

@mnzaki "key": 'names[]' should be "key": 'names'
You only need [] when there is a sub property like names[].names

Because it doesn't know the key it can't replace it with the index.

[mnzaki]

I updated the plunker to reflect that, but now the empty string gets replaced with 0 (for all keys), also form.key seems to be overridden by the last key always (try adding a new value to the array)

[mnzaki]

Also the behavior of sfKeyController seems to be broken for type: "fieldset". The fieldset form addon adds an sf-index="{{$index}}" attribute, but there's no $index defined in that scope and that gets parsed to a 0.

[Anthropic]

@mnzaki I'll take a look tonight, I see the issue as described now thanks :)

[mnzaki]

I have been looking at code, and there seems to be a few problems.
For starters, sfPath.parse("[]") returns [""], is this correct behavior?

If so then in fieldset.html addon template, sf-key-controller sf-parent-key="[{{form.key.join('][')}}]" sf-index="{{$index}}"> will produce incorrect results when form.key is empty, as that evaluates to sf-parent-key="[]" which is parsed to [""] and then the $index (which is empty) is parsed to 0.

[Anthropic]

@mnzaki right, makes sense, I didn't get a chance the other night, wife dragged me out shopping, very much against my will, will definitely take a look this weekend.

[mnzaki]

More analysis:

Another related problem is in this line in sf-field.directive:

        scope.initialForm = Object.assign({}, sfSchema.lookup['f' + attrs.sfField]);
/*->*/  scope.form = sfSchema.lookup['f' + attrs.sfField];

Assigning the form object directly from the sfSchema.lookup will break for arrays as there's only 1 form item object for all the array items, so when changes are made to the object (ie, replacing the "" with the index) it will affect all items. You can see this effect in the plunker, all items have the same form.key (with the indices for the last item processed, as it has replaced the previous ones of course), but the completeKey is correct as it is generated (new object) for each scope.

Changing this to the (IMHO) correct behavior of copying the item each time unfortunately causes breakage in certain areas that depend on the side-effect of form.key being the actual object that is in the lookup, which is also the actual object from inside the form passed to sf-form directive. The things that break are for example this which sets a value on the supplied form object and then expects that this will propagate a change without broadcasting a schemaFormRedraw event.

I think that in a best case scenario this should not be supported (ie, making changes and not firing schemaFormRedraw), but unfortunately it might break existing code that depends on this behavior.

Also note the tests I just pushed and PRed, I purposefully left out the form object everywhere (so that the default is generated) as that triggers broken form.key even for the cases that normally work (like array of objects) if you supply the keys manually.

[mnzaki]

@Anthropic I think I broke something with the fixes related to this issue, please take a look at the test I just added here. When deleting items from an array of objects it deletes other items as well. The bug is triggered when the form object has a "fieldset" (as opposed to the object items directly referenced in the array form object items)

The issue may be faintly related to #750 and #769 and #689

[Anthropic]

Thanks, I am hoping to get some time this week to fix up some of the smaller issues like this. Thanks for the update.

[mnzaki]

@Anthropic please do take a look at #895 which adds a test and a fix for the issue I just mentioned.

[Anthropic]

@mnzaki The should not delete innocent items on delete test is failing, can you take a look at what would be causing that?
ps. it can sometimes be node version related.

[mnzaki]

@Anthropic that's again the same issue because I did not commit dist :D I do remember you said you fixed that but it doesn't look like it's pushed yet.

[Anthropic]

Ah yes, you even added a comment on it to that effect, sorry about that!

[Anthropic]

@mnzaki sat down at my home pc and immediately realised I couldn't sync latest changes as I had the build fix uncommitted, got one bug to fix then I should be able to push it.

[Anthropic]

Merged :)