Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear reactive array #277

Closed
creatormir opened this issue Mar 4, 2024 · 19 comments
Closed

Clear reactive array #277

creatormir opened this issue Mar 4, 2024 · 19 comments

Comments

@creatormir
Copy link

creatormir commented Mar 4, 2024

How to increase the precision of arr.length in van.derive? Currently length == 0 is not possible.

https://jsfiddle.net/creatormir/cbaey9n3/

const {a, button, div, input, span, strike} = van.tags

let items;
const TodoList = () => {
    items = vanX.reactive({arr:[1,2,3], n:555})
    van.derive(() => {
        console.log(items.arr.length, items );
        if (items.arr.length == 0) {
            items.n = 987; // unattainable
        }
    } )
    const inputDom = input({type: "text"})
    return div(()=>items.n)
}

van.add(document.body, TodoList())
items.arr.pop();
items.arr.pop();
items.arr.pop();
@Tao-VanJS
Copy link
Member

Tao-VanJS commented Mar 4, 2024

This seems like a tricky bug in VanX. Handling the length property of reactive arrays is always tricky. I am planning to get this bug fixed as soon as possible.

In the meantime, you can use items.arr = items.arr.slice(0, -1) instead of items.arr.pop(), which is currently working, as you can preview here: https://jsfiddle.net/w1bfpnzq/1/.

@creatormir
Copy link
Author

creatormir commented Mar 5, 2024

Now I got the problem that vanX.list does not work as I expect
https://jsfiddle.net/creatormir/sqnxt0c6/4/

const {a, button, div, input, span, strike} = van.tags

let items;
const TodoList = () => {
    items = vanX.reactive({arr:[0,1,2,3,4,5,6,7,8], n:555})
    van.derive(() => {
        console.log(items.arr.length, items.arr );
        if (items.arr.length == 0) {
            items.n = 987; // unattainable
        }
    } )
    
    return vanX.list(()=>div( items.n), items.arr , (el, deleter, index) =>{
      console.log('vanX.list', index);
      return div(el);
    });
}

van.add(document.body, TodoList());
console.log('\n\n');
items.arr.shift() // vanX.list reload list
items.arr.pop() // vanX.list reload list
items.arr.push(9)
items.arr = items.arr.slice(0, -1); // no reload vanX.list
items.arr = items.arr.slice(0, -1); // no reload vanX.list
items.arr = items.arr.filter((item, idx, array) => idx != 1); // no reload vanX.list # remove elem by idx ~ .splice(idx, 1)

@Tao-VanJS
Copy link
Member

If you're using vanX.list, you should use vanX.replace instead of directly reassigning the value.

@creatormir
Copy link
Author

Thank. This helped, I wrote vanX.replace(items.arr, l => l.filter((item, idx) => idx != 2));

@creatormir
Copy link
Author

creatormir commented Mar 5, 2024

it would be great if vanX.replace returned true if there were changes
let changeCheck = vanX.replace(items.arr, ln => ln.filter(item => item != findValue) );

@creatormir
Copy link
Author

creatormir commented Mar 5, 2024

When using vanX.replace, the original object is not changed (values from the array are not removed). Do I need to remove them separately? In this case, will me need to disable tracking on vanX.list?

@Tao-VanJS
Copy link
Member

When using vanX.replace, the original object is not changed (values from the array are not removed).

I am not sure what you meant here. When using vanX.replace, the items that don't appear in the new list will be removed (together with the corresponding UI elements).

@creatormir
Copy link
Author

Yes, indeed, this problem does not exist now, I forgot to update vanX. However, I noticed that deleter() creates empty cells rather than removing them from the array. To fix this problem, need to repair the array vanX.replace(items.arr, l => l);
https://jsfiddle.net/creatormir/sqnxt0c6/11/

I'm exporting the latest versions, it would be nice to have a van.verion command to know which version is loaded.

<script type="text/javascript" src="https://cdn.jsdelivr.net/gh/vanjs-org/van/public/van-latest.nomodule.js"></script>
<script type="text/javascript" src="https://cdn.jsdelivr.net/npm/vanjs-ext@latest/dist/van-x.nomodule.js"></script>

@Tao-VanJS
Copy link
Member

However, I noticed that deleter() creates empty cells rather than removing them from the array.

This is intended behavior. By allowing "holes" in the array, we don't have to do lots of DOM updates whenever an item is deleted. Basically, you can control when to do the compaction with vanX.replace(items.arr, l => l) while acknowledging that lots of DOM updates will be underway.

@creatormir
Copy link
Author

creatormir commented Mar 5, 2024

I don’t understand well, but what is the logic in leaving empty cells in the array? To fill them or what to do with them? Leave empty until better times? :-)

How can I disable Proxy?
I create a reactive object, make changes to it, and then move it to another array of another reactive object. It is necessary to break the connection between objects. Now I do it like this: mainObj.items.push( JSON.parse(JSON.stringify(newItem)) )
UPD: solution mainObj.items.push( {...emptyItem} )(not helped)

@Tao-VanJS
Copy link
Member

I don’t understand well, but what is the logic in leaving empty cells in the array? To fill them or what to do with them? Leave empty until better times? :-)

We leave cells empty until better timing. Let's say we have a reactive array [1, 2, 3, 4, 5], and the 3rd item is deleted by the user.

If we allow empty cells, the array will become [1, 2, empty, 4, 5]. From the perspective of DOM tree, only the 3rd DOM element needs to be removed.

If we don't allow empty cells, the array will become [1, 2, 4, 5], then we need 3 DOM updates:

  1. 3rd DOM element: 3 -> 4
  2. 4th DOM element: 4 -> 5
  3. Remove the 5th DOM element.

It is necessary to break the connection between objects.

To clarify, by breaking the connection, do you mean to avoid the detection of dependency from one reactive object to another reactive object? Do you have the full piece of code to illustrate your use case?

@Tao-VanJS
Copy link
Member

How to increase the precision of arr.length in van.derive? Currently length == 0 is not possible.

https://jsfiddle.net/creatormir/cbaey9n3/

Hi @creatormir,

FYI, VanX 0.2.5 was just released which fixed the issue of array length binding.

Your example is now working in jsfiddle:
https://jsfiddle.net/odr2nc5x/1/

Thank you so much for reporting the bug!

@Tao-VanJS
Copy link
Member

@all-contributors please add @creatormir for bug

Copy link
Contributor

@Tao-VanJS

I've put up a pull request to add @creatormir! 🎉

@Tao-VanJS
Copy link
Member

I don’t understand well, but what is the logic in leaving empty cells in the array?

@creatormir, one caveat is that because holes might exist in the array, the length property isn't always the number of items in the reactive array. To reliably get the number of items in the array, you should use Object.keys(items).length instead. I am planning to add this caveat to https://vanjs.org/ website as well.

@creatormir
Copy link
Author

creatormir commented Mar 8, 2024

We leave cells empty until better timing.

That is, doing this is incorrect (not effective)?

deleter(); // create empty cell
vanX.replace(items.arr, l => l); // for repair array
//...
van.derive(() => {
  placeholder.hidden = items.arr.length > 0 ;
} );

It's effective to do this?

van.derive(() => {
  placeholder.hidden = items.arr.filter(v=>v).length > 0 ;
} );

or

van.derive(() => {
  placeholder.hidden = Object.keys(items.arr).length > 0 ;
} );

@Tao-VanJS
Copy link
Member

It's effective to do this?

van.derive(() => {
  placeholder.hidden = items.arr.filter(v=>v).length > 0 ;
} );

or

van.derive(() => {
  placeholder.hidden = Object.keys(items.arr).length > 0 ;
} );

Both of the approaches work. The 2nd approach (Object.keys(items.arr).length > 0) might need less computation in theory.

@Tao-VanJS
Copy link
Member

@creatormir, FYI, the caveat was added to VanJS website: https://vanjs.org/x#caveat-array-holes.

@kiruthikpurpose
Copy link

How to increase the precision of arr.length in van.derive? Currently length == 0 is not possible.

https://jsfiddle.net/creatormir/cbaey9n3/

const {a, button, div, input, span, strike} = van.tags

let items;
const TodoList = () => {
    items = vanX.reactive({arr:[1,2,3], n:555})
    van.derive(() => {
        console.log(items.arr.length, items );
        if (items.arr.length == 0) {
            items.n = 987; // unattainable
        }
    } )
    const inputDom = input({type: "text"})
    return div(()=>items.n)
}

van.add(document.body, TodoList())
items.arr.pop();
items.arr.pop();
items.arr.pop();

Nice work, but there's still room for optimization! Instead of relying on items.arr.length where empty cells can skew results, try using items.arr.filter(Boolean).length to ensure you're only counting non-empty values. Alternatively, Object.keys(items.arr).length can give you an even faster result without the need for filtering. This way, you'll handle those pesky array holes more effectively, without any surprises when length hits zero!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants