Options props can be mutated - options behaviour comment in v0.9.1

[dauriata]

Oruga version: 0.9.1

Thanks for all the work done with v0.9 ! I have some questions / comments regarding the new options api in v0.9.1

I would have expected to be able to pass any object as options, and maybe tell what should be the "value" and "label" field, and/or use slot for label.
With version 0.9.1 it seems to be very specific, object are required to have a value property, if not the props are mutated by adding a "hidden" property which seems not good as it can have side effect on the rest of the app.
It looks like it should also not have a key property or it else it would be overwritten.

I am also concerned how this would scale for large datasets ( maybe not for tabs or steps that would not go over 10 entries but certainly for autocomplete where it could be common to search across thousand entries )
I see that all options are rendered in the DOM even when hided, this may not be optimal.
And maybe I don't get all the implication but I also think that using crypto.randomUUID is overkill, I think a simple sequence counter would do if at all needed. Why not use the index of the returning options array as key ?

[holtolee]

For info crypto.randomUUID break the app in Nuxt (crypto.randomUUID is not a function), no issue with 0.9.0-pre.2

[mlmoravek]

Hey and thanks for your feedback!!

And maybe I don't get all the implication but I also think that using crypto.randomUUID is overkill, I think a simple sequence counter would do if at all needed. Why not use the index of the returning options array as key ?

At first I used the useId native Vue composable. But the composables have the restriction of being called in the component setup context. So when a reactive hook is triggered due to an external update of the options prop, the related composable property is recalculated outside the setup context. Therfore I needed an alternative solution to useId. The first and easiest way for me was to use a native browser implementation (like crypto.randomUUID). However, I confirm your problems with nuxt here... and I see your concerns this to be an overkill.
We can try to change it to a simpler sequence counter. Feel free to open a PR :)

[mlmoravek]

Regarding the options prop interface:
I needed a way to add some internal information to the options object. However, I don't want to mutate the real options object as it was done before.
So I adapted the options implementation of the FormKit library (you can see some example usecases here: https://formkit.com/inputs/select

Short explanation of the options prop type:
Most components which use the options prop, the options prop can be defined as follows:

• an array of simple primitives, where the primitive will be the string casted label representation and tthe value.
• an array of objects, where the object is defined as { value: any, label: string, attrs: object }. The value' is the real value object that you access by the v-model prop and other events. The labelis the visible representation of this option. Theattrsis an object that will bev-bind` to the option/item component, which can be used to add additional information depending on the component.
• Some other types can be found here:
  

  oruga/packages/oruga/src/composables/useOptions.ts

  Lines 39 to 42 in a336db2

  	export type OptionsProp<V = string | number | object> =
  	| (string | number)[]
  	| Record<string | number, string>
  	| OptionsPropItem<V>[];

And a more detailed explanation will surely follow in the docs (see #1079).

[mlmoravek]

I see that all options are rendered in the DOM even when hided, this may not be optimal.

For example, with the autocomplete component, having the options already rendered and only hiding them using v-show when the string to autocomplete does not match the option is much more performant than removing them from the DOM.

[pflirae]

For info crypto.randomUUID break the app in Nuxt (crypto.randomUUID is not a function), no issue with 0.9.0-pre.2

related to #1105

[mlmoravek]

@dauriata Can this be closed? Or do you have any more question?

[dauriata]

Everything might not be relevant but to follow the discussion I'll share my use case and my thoughts :

I have a customer dataset in store as an array of thousand objects with property id, firstname, name. I used to have a search field to select id as an autocomplete with a slot display value of the kind {{firstname name}} .

Doing so is not possible to tell the select field is id, I have to make a copy and name it value.
Actually if it was value, the original array would be mutated by adding hidden property which I would not want because this array is used elsewhere. If it is intended behavior I think it should be stated in docs.
( side note I've seen a similar behavior in tables where a _rowKey field was added to my input props. )

So I think the only solution for my use case is to craft a special copy of my options array. Going down the code I notice there are two more copies of the data inside the component, a tree version as groupeOptions and a flat version toOptionsList for key navigation. I notice groups are only one level deep, and it is not possible to have items outside of a group at root level.
It seems that groupOptions are only used in dropdown and autocomplete, so why not only use a single flatenned version of the dataset with group header tagged with a group property to style them and skip them in selection, I think it would made all code simpler.

Performance wise when input dataset might be large, I think it is good to have a versatile api that can directly use input data without duplication.
My dreamed api would be when options are an array of objects to :

• have a prop to tell what field to use as select field
• have a prop to tell what field to use as label or use default slots
• no mutation of props in any case
• minimize data duplication, map to original input data when it's possible

Another note off topic, I think the clear button should also clear input search text and not only the selected item.

[mlmoravek]

I appreciate your feedback and your deep thoughts on the topic!

no mutation of props in any case

First, this implementation is specifically used to not mutate the data passed in the options prop. This requires us to create a new internal object for each option, to which additional information can be added, such as a unique key/id or hidden state information, etc... . The mapping of the options prop data to the internal representation (usually called groupedOptions in the code) is done once at setup and every time the options prop changes. However, the options prop shouldn't change that often.

minimize data duplication, map to original input data when it's possible

I was trying to find a pattern that could be used by any component that has an options prop, to simplify the different component uses, instead of having different options/data implementations for each component as before.

Also, the groupedOptions variable in the code is only used if the options prop is set. If you prefer to use child components in the template, such as ODropdownItem, OTableColumn, etc., the options won't be used. However, I know that some components only work with the options prop and don't have a child component like OAutocomplete and OTaginput.

have a prop to tell what field to use as select field

The value attribute of an option defines this.

have a prop to tell what field to use as label or use default slots

The label attribute of an option defines this. The implementation of a slot override depends on the component.

Another note off topic, I think the clear button should also clear input search text and not only the selected item.

Sure, this is a bug that I have encountered myself. Will be fixed in the future.

[dauriata]

this implementation is specifically used to not mutate the data passed in

Here is an example codesandbox, that happen when there are no value field, if that's excluded maybe it's better to give an error.

The value attribute of an option defines this.

Yes but my all point was to be able to use input data as they are, not having to make a copy with specific attributes names when the dataset is big.

I am no expert but would not it be preferable to use an internal object that ref the initial array objects with added custom attributes like { option, key: xxx, hidden: false } instead of making a shallow copy : {...option, key: xxx, hidden: false} , again having in mind the input data can be large or have lots of properties.

[mlmoravek]

Here is an example codesandbox,

Sorry, I can't open your example...

that happen when there are no value field, if that's excluded maybe it's better to give an error.

Normally there should be a type error if the value attribute is not set. Maybe check your typescript configuration?

Yes but my all point was to be able to use input data as they are, not having to make a copy with specific attributes names when the dataset is big.

I understand your point. But I think pre-processing your options isn't that big of a deal with computers these days, and if your dataset is that huge, maybe you should consider thinking about your data and application structure instead?

I am no expert but would not it be preferable to use an internal object that ref the initial array objects with added custom attributes like { option, key: xxx, hidden: false } instead of making a shallow copy : {...option, key: xxx, hidden: false} , again having in mind the input data can be large or have lots of properties.

Hmm, I will think about it. However, Oruga doesn't touch and mutate the value attribute of the option, which is the real value of the modelValue prop you get anyway. Only the wrapper option item object is restructured with additional information (key, etc..). This is the same as mapping the attributes one by one from one object to another, like {label: option.label, value: option.value, attrs: option.attrs, key: uuId(), ... }.