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

[Bug] On change of a {{each}}-looped array, Ember removes components elements from DOM but does not destroy components #19020

Closed
gabrielgrant opened this issue Jun 18, 2020 · 10 comments

Comments

@gabrielgrant
Copy link

gabrielgrant commented Jun 18, 2020

🐞 Describe the Bug

I have an {{each}} looping over an array and creating a component for each of the array entries. When the array changes for the second (and subsequent) times, Ember removes the DOM elements generated by the components but does not destroy components.

The array is returned from getter which slices a larger array (so the returned arrays are unique per-retrieval). The entries of the larger array are unique objects.

🔬 Minimal Reproduction

Haven't yet come up with a minimal repro, but the problem can be observed with the following steps:

  1. visit https://bb.gabrielgrant.ca/search-map/restaurants/around/Vancouver,%20BC,%20Canada?perPage=2 (perPage set artificially low to make it easier to observe the DOM/component hierarchy visually)
  2. observe that there are only two entries in the list and on the map
  3. click to page 2 (link at the bottom)
  4. observe that there are again only two entries in the list and on the map
  5. click to page 3
  6. observe that there are still only two entries in the list, but four on the map

😕 Actual Behavior

After following the steps above, ember inspector shows:

  1. there are still four RestaurantCard components present, although only two appear in the DOM
  2. there are also four marker components (the two from before would have been removed had the component been destroyed: https://github.com/kturney/ember-mapbox-gl/blob/master/addon/components/mapbox-gl-marker.js#L61 )
  3. the backing array (AroundController.paginatedResults) only contains two elements

🤔 Expected Behavior

When the backing array changes, Components should both be removed from the DOM and destroyed

🌍 Environment

$ ember version --verbose
ember-cli: 3.17.0
http_parser: 2.8.0
node: 10.16.3
v8: 6.8.275.32-node.54
uv: 1.28.0
zlib: 1.2.11
brotli: 1.0.7
ares: 1.15.0
modules: 64
nghttp2: 1.39.2
napi: 4
openssl: 1.1.1c
icu: 64.2
unicode: 12.1
cldr: 35.1
tz: 2019a
os: linux x64

➕ Additional Context

This issue was originally reported on the ember-mapbox-gl repo. There's lots more info in that thread: kturney/ember-mapbox-gl#91

@kturney suggested it may have something to do with the complexity around key described in
this Ember issue, but I don't believe that's the case, for the reasons described in this comment: kturney/ember-mapbox-gl#91 (comment)

@rwjblue
Copy link
Member

rwjblue commented Jun 18, 2020

@gabrielgrant - This was fixed and released in 3.19.0, I think.

@rwjblue
Copy link
Member

rwjblue commented Jun 18, 2020

Can you check?

@gabrielgrant
Copy link
Author

gabrielgrant commented Jun 18, 2020

Ah, didn't even realize there was a 3.19 yet. Will do!

Edit: ah, i see -- ember-cli 3.19 isn't cut yet, but ember proper is

@JackieChiles
Copy link

I filed #19033 which seems like it could be related to this, but my example shows leaking DOM nodes and memory with a very simple reproduction. It does not render any components in the loop.

@rwjblue
Copy link
Member

rwjblue commented Jun 30, 2020

@gabrielgrant - Did you confirm if Ember 3.19 resolved this issue?

@gabrielgrant
Copy link
Author

Sorry, have had a few other things on my plate, so haven't gotten around to upgrading yet. Hopefully should be able to do so today or tomorrow

@rwjblue
Copy link
Member

rwjblue commented Jun 30, 2020

Awesome, thank you. Please let us know, as I'd like to confirm that this is resolved or if we still need to debug...

@rwjblue
Copy link
Member

rwjblue commented Jul 15, 2020

I can't tell if this is the same as what we fixed for #19033, that fix will be released in v3.20.1 shortly.

@gabrielgrant
Copy link
Author

gabrielgrant commented Jul 17, 2020

@rwjblue Thanks for following up on this again, and sorry (again) for taking ages to get back to it. Adding key="@index" to the {{each}} enclosing the map markers had seemed to function as a workaround (even though, from my understanding, it shouldn't have behaved any differently). Just finally got around to upgrading to 3.19, though, and it seems that I'm able to remove that key without causing the original issue to reappear. So it seems this is resolved :)

Curious if you recall what issue this you'd guessed this was related to that was shipped in 3.19.0? (seems it wasn't #19033, since the fix was in 3.19)

@rwjblue
Copy link
Member

rwjblue commented Jul 21, 2020

@gabrielgrant I think it was #18982

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

3 participants