search

koumoul-dev / vuetify-jsonschema-form

Create beautiful and low-effort forms that output valid data. Published on npm as @koumoul/vjsf.
https://koumoul-dev.github.io/vuetify-jsonschema-form/latest/
MIT License
538 stars 154 forks source link

Conditionalize prefilled arrays #284

Closed mvandenburgh closed 2 years ago

mvandenburgh commented 2 years ago

This PR adds a new prop, disablePrefilledArrays, that disables prefilled arrays in vjsf.

We're using vjsf for https://github.com/dandi/dandiarchive and we've been running into performance issues with large array fields (original issue for reference). We did some profiling on vjsf and determined that removing this code results in a significant performance boost. That code is related to loading "prefilled arrays", which our application does not make use of at the moment. Due to the significant performance overhead caused by this feature, we want to propose making it toggleable by a prop passed to vjsf (see the changes to the documentation for details).

Here are two examples of datasets that load slowly with vjsf 2.1.3. I've also included examples of those same datasets using a modified version of vjsf with the previously mentioned code removed.

Dandiset 4 (vjsf 2.1.3, forked vjsf with performance boost)

Dandiset 26 (vjsf 2.1.3, forked vjsf with performance boost)

albanm commented 2 years ago

Sorry for the lack of response recently. I am on holidays, i will have a look next week.

albanm commented 2 years ago

I won't merge this as it is, at least not straightaway as I would like to obtain the same gain without adding an option. But the diagnostic is very helpful anyway, thanks for digging.

albanm commented 2 years ago

To clarify, there is an ambiguity with the expression "prefilled array". In the doc it references arrays filled using external data (http fetch mostly), in the code you modified it simply is about arrays existing in the model when initializing vjsf. The purpose of this code is to apply to the model the automatic fixes done by vjsf on the items (setting default value, setting const value, removing additional properties).

I have seen 1 discrepancy in your example that might explain why this is triggered en masse, there might be others : includeInCitation is present many times in the model inside affiliation but not in the schema therefore it is removed automatically (add it to the schema or set option removeAdditionalProperties=false to change this behavior).

Nevertheless I merge this PR as I am ok with making this functionality optional, but I will rename the new option before releasing (autoFixArrayItems I think).