search

lit / lit

Lit is a simple library for building fast, lightweight web components.
https://lit.dev
BSD 3-Clause "New" or "Revised" License
18.73k stars 923 forks source link

[labs/virtualizer] Renders the entire list when invisible #4672

Closed Sleeckx closed 5 days ago

Sleeckx commented 4 months ago

Which package(s) are affected?

Virtualizer (@lit-labs/virtualizer)

Description

I have a deeply nested lit-virtualizer element in my web app that renders all items whenever one of its parent elements becomes invisible.

I believe the viewport height calculation in the _updateView function is sometimes incorrect. In my case, the calculated height becomes negative, which causes all items to get rendered. Changing the calculation to const height = Math.max(0, bottom - top) resolves the issue and the component works as intended.

Reproduction

https://lit.dev/playground/#project=W3sibmFtZSI6InNpbXBsZS1saXN0LnRzIiwiY29udGVudCI6ImltcG9ydCB7aHRtbCwgY3NzLCBMaXRFbGVtZW50fSBmcm9tICdsaXQnO1xuaW1wb3J0IHtjdXN0b21FbGVtZW50LCBwcm9wZXJ0eSwgcXVlcnl9IGZyb20gJ2xpdC9kZWNvcmF0b3JzLmpzJztcbmltcG9ydCAnQGxpdC1sYWJzL3ZpcnR1YWxpemVyJztcblxuQGN1c3RvbUVsZW1lbnQoJ3NpbXBsZS1saXN0JylcbmV4cG9ydCBjbGFzcyBTaW1wbGVMaXN0IGV4dGVuZHMgTGl0RWxlbWVudCB7XG4gIHN0YXRpYyBzdHlsZXMgPSBjc3NgXG4gICAgICA6aG9zdCB7IGRpc3BsYXk6IGZsZXg7IGZsZXgtZGlyZWN0aW9uOiBjb2x1bW47IG1heC1oZWlnaHQ6IDQwMHB4OyB9XG4gICAgICBbaGlkZGVuXSB7IGRpc3BsYXk6IG5vbmUgIWltcG9ydGFudDsgfVxuICAgICAgLnBhcmVudCB7IGZsZXg6IDE7IG92ZXJmbG93OiBoaWRkZW47IGRpc3BsYXk6IGZsZXg7IGZsZXgtZGlyZWN0aW9uOiBjb2x1bW47IH1cbiAgICAgIG1haW4geyBvdmVyZmxvdzogYXV0bzsgYm9yZGVyOiAxcHggc29saWQgZ3JlZW47IH1cbiAgICAgIGA7XG4gIFxuICBfaW50ZXJ2YWw6IG51bWJlcjtcbiAgXG4gIEBwcm9wZXJ0eSgpXG4gIGl0ZW1zID0gQXJyYXkuZnJvbSh7IGxlbmd0aDogMTAwMCAtIDEgKyAxIH0sIChfLCBpKSA9PiBpICsgMSk7XG4gIFxuICBAcHJvcGVydHkoeyB0eXBlOiBOdW1iZXIgfSlcbiAgY2hpbGRDb3VudCA9IDA7XG4gIFxuICBAcXVlcnkoXCJsaXQtdmlydHVhbGl6ZXJcIilcbiAgbGlzdDtcbiAgXG4gIEBwcm9wZXJ0eSgpXG4gIGhpZGVMaXN0ID0gZmFsc2U7XG4gIFxuICBjb25uZWN0ZWRDYWxsYmFjaygpIHtcbiAgICBzdXBlci5jb25uZWN0ZWRDYWxsYmFjaygpO1xuICAgIFxuICAgIHRoaXMuX2ludGVydmFsID0gc2V0SW50ZXJ2YWwoKCkgPT4ge1xuICAgICAgdGhpcy5jaGlsZENvdW50ID0gdGhpcy5saXN0LnF1ZXJ5U2VsZWN0b3JBbGwoXCJkaXZcIikubGVuZ3RoO1xuICAgIH0sIDI1MCk7XG4gIH1cblxuICBkaXNjb25uZWN0ZWRDYWxsYmFjaygpIHtcbiAgICBzdXBlci5kaXNjb25uZWN0ZWRDYWxsYmFjaygpO1xuICAgIGNsZWFySW50ZXJ2YWwodGhpcy5faW50ZXJ2YWwpO1xuICB9XG5cbiAgcmVuZGVyKCkge1xuICAgIHJldHVybiBodG1sYFxuICAgICAgICA8YnV0dG9uIEBjbGljaz0ke3RoaXMuX2hpZGV9PlRvZ2dsZSB2aXNpYmxlPC9idXR0b24-XG4gICAgICAgIDxkaXY-TnVtYmVyIG9mIHJlbmRlcmVkIGl0ZW1zOiAke3RoaXMuY2hpbGRDb3VudH08L2Rpdj5cbiAgICAgICAgPGRpdiBjbGFzcz1cInBhcmVudFwiPlxuICAgICAgICAgIDxtYWluIC5oaWRkZW49JHt0aGlzLmhpZGVMaXN0fT5cbiAgICAgICAgICAgIDxsaXQtdmlydHVhbGl6ZXJcbiAgICAgICAgICAgICAgLml0ZW1zPSR7dGhpcy5pdGVtc31cbiAgICAgICAgICAgICAgLnJlbmRlckl0ZW09JHsoaXRlbSkgPT4gaHRtbGA8ZGl2IGNsYXNzXCJpdGVtXCI-JHtpdGVtfTwvZGl2PmB9XG4gICAgICAgICAgICA-PC9saXQtdmlydHVhbGl6ZXI-XG4gICAgICAgICAgPC9tYWluPlxuICAgICAgICA8L2Rpdj5gO1xuICB9XG4gIFxuICBfaGlkZSgpIHtcbiAgICB0aGlzLmhpZGVMaXN0ID0gIXRoaXMuaGlkZUxpc3Q7XG4gIH1cbn1cbiJ9LHsibmFtZSI6ImluZGV4Lmh0bWwiLCJjb250ZW50IjoiPCFET0NUWVBFIGh0bWw-XG48aGVhZD5cbiAgPHNjcmlwdCB0eXBlPVwibW9kdWxlXCIgc3JjPVwiLi9zaW1wbGUtbGlzdC5qc1wiPjwvc2NyaXB0PlxuPC9oZWFkPlxuPGJvZHk-XG4gIDxzaW1wbGUtbGlzdD48L3NpbXBsZS1saXN0PlxuPC9ib2R5PlxuIn0seyJuYW1lIjoicGFja2FnZS5qc29uIiwiY29udGVudCI6IntcbiAgXCJkZXBlbmRlbmNpZXNcIjoge1xuICAgIFwibGl0XCI6IFwiXjMuMC4wXCIsXG4gICAgXCJAbGl0L3JlYWN0aXZlLWVsZW1lbnRcIjogXCJeMi4wLjBcIixcbiAgICBcImxpdC1lbGVtZW50XCI6IFwiXjQuMC4wXCIsXG4gICAgXCJsaXQtaHRtbFwiOiBcIl4zLjAuMFwiXG4gIH1cbn0iLCJoaWRkZW4iOnRydWV9XQ

Workaround

I have not found a workaround without changing the lit-virtualizer code as described above.

Is this a regression?

Yes. This used to work, but now it doesn't.

Affected versions

Failing for 2.0.11 and above

Browser/OS/Node environment

Browsers tested and failing on

graynorton commented 4 months ago

@Sleeckx Thank you for the clean, minimal repro! I will take a look.

I would like to make sure I understand why the height calculation ends up being negative before applying the suggested fix, but in any case I am confident we can get this resolved fairly quickly.