fix: [Table] #295 data reactivity issue

[mattgoud]

❓ Types of changes

• 📖 Documentation (updates to the documentation or readme)
• 🐞 Bug fix (a non-breaking change that fixes an issue)
• 👌 Enhancement (improving an existing functionality)
• 📦 New feature (a non-breaking change that adds functionality)
• ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

data reactivity issue

📝 Checklist

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes
• The component exists on old Prestashop UIKit and my pull request on migrating documentation is accepted.

[kktos]

this is a readonly var so it should be better to have :

const data = toRef(() => props.items);

[kktos]

and I just realised why there was a copy of the data here. The sort !
So as you're mutating the value, you made a copy of it beforehand.
That could be costly, depending on the data size.
It could be worth having an option for that: allowsToMutate or not.

[kktos]

there is a typo:
PuikTableScrollBarPosistion
should be
PuikTableScrollBarPosition
table.ts#L16

[mattgoud]

yes that's what I did but it generates errors for me, the function expects arguments however it seems to respect the signature as in the example in the doc:

// creates a readonly ref that calls the getter on .value access
toRef(() => props.foo)

I will use watch() as an alternative

[mattgoud]

and I just realised why there was a copy of the data here. The sort ! So as you're mutating the value, you made a copy of it beforehand. That could be costly, depending on the data size. It could be worth having an option for that: allowsToMutate or not.

yes, it can be expensive, otherwise there is a sortFromServer prop which prevents sorting on the client side. Only a SortOptions type payload is sent (which can be useful for building a server-side query instead)

[mattgoud]

yes that's what I did but it generates errors for me, the function expects arguments however it seems to respect the signature as in the example in the doc:

// creates a readonly ref that calls the getter on .value access
toRef(() => props.foo)

I will use watch() as an alternative

ha! it must be because the project was still in 4.2.47...

[kktos]

Each time you find a bug, it is good practice to add a UT for it.
Here, you may add a test with items =[] then change it and see if the table is updated.
It'll be a good guard against regression.
;)

[mattgoud]

Each time you find a bug, it is good practice to add a UT for it. Here, you may add a test with items =[] then change it and see if the table is updated. It'll be a good guard against regression. ;)

yes @kktos, done