FF Mobile & Safari - ScrollY binding performance issue

[andreujuanc]

Hi,

I took a template and I removed all the jQuery and redoing everything in svelte+sapper. This is just a POC to give as a starting base to a team who's going to actually make it.

Link: https://wgmsacom-hyuydhwlgb.now.sh/

It's using <svelte:window bind:scrollY=scrollY/> and then to calculate when it's on certain scroll position.

       onstate({ changed, current, previous }) {
        if (changed.scrollY) {
                this.set({
                    isTransparent: current.scrollY < 65
                });
            }
        }

Works fine everywhere, but it used to be slow on my S8 when i was using a computed property to do the calc, after that, replaced it with onstate and got it smooth everywhere, except in the iPhone SE that i have available (thanks hun) where it runs like a potato 🥔.

I removed the binding and scrolls up and down smoothly again.

My question, Can it be an issue with iOS, Safari and/or the scroll binding?

PD: if the link is down, it's my fault. Just ask and I will now it ASAP.

Thanks

[andreujuanc]

Copy the RELP paralax demo and published here: https://sveltedemo-gzufesoawc.now.sh/ Run it in my S8 with Firefox and Chrome.

FF: choppy, slow, and not fluent Chrome: nicely smooth.

[maxmilton]

I'm not sure that binding directly to scrollY is the best way to go about this. Typically for reacting to viewport scrolling you would listen to the document scroll event and use some kind of debounce since scroll triggers too quickly.

I recently done something along these lines in a navbar component to detect if a user is at the top of the page or has scrolled: https://github.com/WeAreGenki/minna-ui/blob/master/components/navbar/src/MinnaNavbar.html#L106

[andreujuanc]

Hi Max, I see. It´s more or less what I´m doing now.

But I saw somewhere that binding to scroll was the way to go. Specially to simplify code and make it cleaner.

I´m pretty sure that it is an implementation issue with Firefox mobile, since this demo: https://svelte.technology/repl?version=2.9.3&demo=parallax

Works great in my phone using Chrome , but no other web browser. So it´s not a matter of whenever binding or not it´s a good idea, but more like, what is that binding doing under the hood on each browser.

Anyway, I might just patch it with code similar of what you did in that link.

Cheers

[arggh]

I just noticed that plain simple adding this...

<svelte:window bind:scrollY=y/>

...to a component causes massive scroll jank on iPhones. Even if I'm not using that y-variable anywhere.

I don't think it's relevant to say "you didn't use throttle", because when replaced with this...

oncreate() {
    this.listener = () => this.set({ y: window.pageYOffset });
    window.addEventListener('scroll', this.listener);
}

...scrolling is again smooth as butter. When checking from the REPL, the code generated by that binding does have some kind of scrolljacking logic in it. That would be my first guess.

[mrkishi]

Huh, the generated code seems... Broken? Unless I'm misunderstanding it, it looks like it's calling window.scrollTo in response to scroll events. I don't get what the purpose even was here?

    // <svelte:window bind:scrollY="y"/>
    function onwindowscroll(event) {
        if (window_updating) return;
        window_updating = true;
        component._updatingReadonlyProperty = true;

        component.set({
            y: this.pageYOffset
        });

        component._updatingReadonlyProperty = false;
        window_updating = false;
    }
    window.addEventListener("scroll", onwindowscroll);

    component.on("state", ({ changed, current }) => {
        if (changed["y"]) {
            window_updating = true;
            clearTimeout(window_updating_timeout);
            window.scrollTo(window.pageXOffset, current["y"]);
            window_updating_timeout = setTimeout(clear_window_updating, 100);
        }
    });

[arggh]

@mrkishi I had a hard time understanding the reasoning behind this as well.

[arggh]

This is the old version of relevant compiler code from 1.64.1:

if (bindings.scrollX && bindings.scrollY) {
  const observerCallback = block.getUniqueName(`scrollobserver`);

  block.builders.init.addBlock(deindent`
    function ${observerCallback}() {
      ${lock} = true;
      clearTimeout(${timeout});
      var x = ${bindings.scrollX
        ? `#component.get("${bindings.scrollX}")`
        : `window.pageXOffset`};
      var y = ${bindings.scrollY
        ? `#component.get("${bindings.scrollY}")`
        : `window.pageYOffset`};
      window.scrollTo(x, y);
      ${timeout} = setTimeout(${clear}, 100);
    }
  `);

  if (bindings.scrollX)
    block.builders.init.addLine(
      `#component.observe("${bindings.scrollX}", ${observerCallback});`
    );
  if (bindings.scrollY)
    block.builders.init.addLine(
      `#component.observe("${bindings.scrollY}", ${observerCallback});`
    );
} else if (bindings.scrollX || bindings.scrollY) {
  const isX = !!bindings.scrollX;

  block.builders.init.addBlock(deindent`
    #component.observe("${bindings.scrollX || bindings.scrollY}", function(${isX ? 'x' : 'y'}) {
      ${lock} = true;
      clearTimeout(${timeout});
      window.scrollTo(${isX ? 'x, window.pageYOffset' : 'window.pageXOffset, y'});
      ${timeout} = setTimeout(${clear}, 100);
    });
  `);
}

...and here is the new one...

if (bindings.scrollX || bindings.scrollY) {
  block.builders.init.addBlock(deindent`
    #component.on("state", ({ changed, current }) => {
      if (${
        [bindings.scrollX, bindings.scrollY].map(
          binding => binding && `changed["${binding}"]`
        ).filter(Boolean).join(' || ')
      }) {
        ${lock} = true;
        clearTimeout(${timeout});
        window.scrollTo(${
          bindings.scrollX ? `current["${bindings.scrollX}"]` : `window.pageXOffset`
        }, ${
          bindings.scrollY ? `current["${bindings.scrollY}"]` : `window.pageYOffset`
        });
        ${timeout} = setTimeout(${clear}, 100);
      }
    });
  `);
}

They both seem to have the same oddity.

[Rich-Harris]

scrollY is a two-way binding — if you set the scroll position programmatically (e.g. you're tweening the scroll position to the next 'checkpoint', or whatever) then window.scrollY should change.

See this fork of the parallax example to see it in action (change the value in the top left).

when replaced with this...

oncreate() {
    this.listener = () => this.set({ y: window.pageYOffset });
    window.addEventListener('scroll', this.listener);
}

...scrolling is again smooth as butter. When checking from the REPL, the code generated by that binding does have some kind of scrolljacking logic in it. That would be my first guess.

Well that's odd! That's basically all the built-in scroll binding is doing, as far as I can tell — what am I missing?

[mrkishi]

Probably something like this?

// <svelte:window bind:scrollY="y"/>
    function onwindowscroll(event) {
        if (window_updating) return;
        window_updating = true;
        component._updatingReadonlyProperty = true;

        component.set({
            y: this.pageYOffset
        });

        component._updatingReadonlyProperty = false;
        window_updating = false;
    }
    window.addEventListener("scroll", onwindowscroll);

    component.on("state", ({ changed, current }) => {
--      if (changed["y"]) {
++      if (changed["y"] && !window_updating) {
            window_updating = true;
            clearTimeout(window_updating_timeout);
            window.scrollTo(window.pageXOffset, current["y"]);
            window_updating_timeout = setTimeout(clear_window_updating, 100);
        }
    });

[Rich-Harris]

oh, right!

[andreujuanc]

I see! I was not crazy after all!!! So it was just not being throttled?

[arggh]

@andreujuanc the two-way binding was a little too eager. The state change listener did not check if the value was just being updated due to scrolling event.

[arggh]

I had to verify.