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

feat: ArrayObserver... intercept array.length assignment and notify subscribers #465

Open
jdanyow opened this issue Jul 27, 2016 · 8 comments

Comments

@jdanyow
Copy link
Contributor

jdanyow commented Jul 27, 2016

@ZoolWay commented on Wed Jul 27 2016

I'm submitting a bug report

  • Library Version:
    aurelia-framework 1.0.0-rc.1.0.13

Please tell us about your environment:

  • Operating System:
    Windows 10
  • Node Version:
    6.3.1
  • NPM Version:
    3.10.3
  • JSPM OR Webpack AND Version
    JSPM 0.16.39
  • Browser:
    Chrome 51.0.2704.103 m
  • Language:
    all

Current behavior:
When I reset an array by setting its length to 0 its items in a repeat.for are no longer removed.

This example shows three possibilities to clear:
https://gist.run/?id=4caf35a8abc1cedfc8487df1ea997548

Splice and replace still work, setting length not. When you try to clear with setting length to 0 a splice after that call will also not work (while replacing still would)!

Note: I have experimented using the TaskQueue because at first I though it was a problem with that.

Expected/desired behavior:
This was working in aurelia-framework 1.0.0-rc.1.0.0. When setting the length to zero and adding new items, the list was cleared and rebuild as expected.

@jdanyow jdanyow changed the title Clearing arrays with length=0 no longer working feat: ArrayObserver... intercept array.length assignment and notify subscribers Jul 27, 2016
@jdanyow
Copy link
Contributor Author

jdanyow commented Jul 27, 2016

cc @martingust @EisenbergEffect

this might be a nice interoperability improvement. thoughts?

@EisenbergEffect
Copy link
Contributor

I think so. For some reason I thought we already did that but it must be only handling bindings to the length.

@BringerOD
Copy link

BringerOD commented Aug 27, 2016

No sure if this is related, and it involves "repeat.for" and same versions "1.0.0" and "1.0.0-rc.1.0.0". Yesterday I move my code base from the JSPM style in the skeleton project, to the new aurelia cli.

During the move the versions were updated to "1.0.0".

After all of it compiled and ran, everything worked except the "repeat.for" no longer was detecting the removal of items in the array, for my tab interface. I am using lodash to remove from the array.

I have a small example of this working and not working with the code base on github.

To run the demo of it working click here and the code on github click here.

To see the exact same code not working click here and the code on github click here.

I am going to play around with removing the object from the array a couple of different ways to see if it detects the change.

Also, side note, is there a better way to build a multi-tab interface.

After some help from "Patrick Walters @PWKad" I came up with what you see in the demos.

Here was my original question.

http://stackoverflow.com/questions/39023359/ability-to-create-a-multi-tab-interface-with-multiple-views-open-at-the-same-ti

Update 8/27/2016:
Using splice works, using _.remove from lodash does not work.

@Alexander-Taran
Copy link
Contributor

Alexander-Taran commented Mar 3, 2018

Something probably done about this one..
or no one cares anymore
https://gist.run/?id=65185f8f61081ffc763c526fc1dc89ad

setting length to 0 still does not trigger update

@EisenbergEffect
Copy link
Contributor

I think it's still a reasonable improvement, just not high priority.

@fkleuver
Copy link
Member

fkleuver commented Aug 31, 2018

Just FYI, the only proper way to do this is with proxies, but an alternative would be to add a dirty checker that translates .length change into a .splice call See #710 (comment)

@Alexander-Taran
Copy link
Contributor

@fkleuver what would be the vNext strategy on this?

@fkleuver
Copy link
Member

fkleuver commented Jan 7, 2019

This is a "future" for vNext. My plan is to add additional binding strategies besides the regular defineProperty get/set one. One will be fully dirty-checking ("react style") and the other will be with proxies. But it's pretty low on the priority list at the moment. It's important, but other things are just more important. Definitely add it to vNext todo's however.

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

No branches or pull requests

5 participants