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

<:Window> component #371

Closed
Rich-Harris opened this issue Mar 14, 2017 · 35 comments
Closed

<:Window> component #371

Rich-Harris opened this issue Mar 14, 2017 · 35 comments

Comments

@Rich-Harris
Copy link
Member

I often find myself doing this sort of thing:

<div><!-- stuff --></div>

<script>
  export default {
    data () {
      return {
        width: window.innerWidth,
        height: window.innerHeight
      };
    },

    oncreate () {
      const onresize = () => this.resize();
      window.addEventListener( 'resize', onresize, false );
  
      this.on( 'teardown', () => {
        window.removeEventListener( 'resize', onresize, false );
      });
    },

    methods: {
      resize () {
        this.set({
          width: window.innerWidth,
          height: window.innerHeight
        });
      }
    }
  };
</script>

Lots of boilerplate, and it's easy to forget to remove the event listener when the component is destroyed.

Occurs to me we could have a special <:Window/> component (using the convention started with <:Self/>), that behaved thusly:

<:Window on:resize='resize()'/>
<div><!-- stuff --></div>

<script>
  export default {
    data () {
      return {
        width: window.innerWidth,
        height: window.innerHeight
      };
    },

    methods: {
      resize () {
        this.set({
          width: window.innerWidth,
          height: window.innerHeight
        });
      }
    }
  };
</script>

Or even this...

<:Window on:resize='set({ width: this.innerWidth, height: this.innerHeight })'/>
<div><!-- stuff --></div>

<script>
  export default {
    data () {
      return {
        width: window.innerWidth,
        height: window.innerHeight
      };
    }
  };
</script>

Or even this:

<:Window bind:innerWidth='width' bind:innerHeight='height'/>
<div><!-- stuff --></div>

Lots of other events that would be useful — on:offline, on:hashchange and so on. A lot of them could have their own bindings: bind:hash, bind:online, bind:scrollTop (in fact that one could apply to regular elements, not just window) and so on.

Similarly:

<:Document on:fullscreen='set({ isFullscreen: this.fullscreen })'/>

<!-- or... -->
<:Document bind:fullscreen='isFullscreen'/>

The glorious thing about being fully compiled is we can kind of go nuts with this stuff without anyone needing to install extra components or ship code they don't need. The only cost we need to worry about is documentation and maintenance cost.

Interested to hear what people think of these ideas.

@PaulBGD
Copy link
Member

PaulBGD commented Mar 14, 2017

I like the idea because it heavily lowers the amount of code to track data from the window/document, but it is very confusing to mix "meta" tags with UI tags.

@Rich-Harris
Copy link
Member Author

@PaulBGD do you mean the confusion between <:Self/> and <:Window/>, or just the fact that 'meta' tags exist in the same place as UI tags? Any ideas for how to make it less confusing? (We could enforce certain rules, e.g. they can't appear inside elements or {{#each}} blocks, and you can only have one of each type per component)

@PaulBGD
Copy link
Member

PaulBGD commented Mar 15, 2017

What I mean is, <:Window/> describes information about the page where as a <p> describes the page itself. Currently the HTML structure of a Svelte component is very pure and doesn't handle much logic, allowing you to put that inside of the JS. <:Window/> or <:Document/> just feels like putting the logic in the wrong place.

@Rich-Harris
Copy link
Member Author

Is it any different in that regard from <link> or <style> or <meta> or <title> etc?

@PaulBGD
Copy link
Member

PaulBGD commented Mar 15, 2017

Those tags aren't really used in components, not to mention on an HTML page they're put in the header - separated from the UI.

@Rich-Harris
Copy link
Member Author

You can put <link>, <style> and <script> anywhere, but even if they're in the <head> the point is that web developers are well-used to non-UI things appearing between angle brackets in an HTML context. Is there really such a risk of confusion? Does it outweigh the benefits of being able to use those tags? It could save typing a lot of code...

@PaulBGD
Copy link
Member

PaulBGD commented Mar 15, 2017

I just think that there's a better way, like moving this into the JS.

@Rich-Harris
Copy link
Member Author

Do you have an example of what that would look like? Not trying to be awkward, just struggling to understand what you mean by that

@Conduitry
Copy link
Member

I think the suggested method of handling this via special components is a pretty tidy way of doing this. Yeah it's a little bit of magic, but I do think it's worth it. I'm thinking that besides letting us bind to events on window and document, this would also let us, say, update document.title.

I'm a little concerned about how two-way binding would work here. I'd probably like as few special cases on the <:Window> and <:Document> elements as possible. That is, using on: directives would bind directly to that event on the window/document, and Svelte doesn't need to know about what the possible events are. And other attributes would just set that property on the window/document. Getting two-way binding to work on given properties seems maybe a little too magical, and I imagine it would require different custom code generated by Svelte for watching changes to each property, and probably some properties would be impossible to watch without polling.

@Rich-Harris
Copy link
Member Author

Yeah, the event listeners is the easy win. The binding thing actually only occurred to me as I was writing this issue. It would be a special case for each different binding — for example the width and height bindings might add code like so:

<:Window bind:innerWidth='width' bind:innerHeight='height'/>
function SvelteComponent ( options ) {
  options = options || {};
  this._state = options.data || {};
+  this._state.width = window.innerWidth;
+  this._state.height = window.innerHeight;

+  window.addEventListener( 'resize', this._windowResizeHandler = function () {
+    this.set({ width: this.innerWidth, height: this.innerHeight });
+  });
  // ...
}

// ...

SvelteComponent.prototype.destroy = function destroy ( detach ) {
  this.fire( 'teardown' );

  this._fragment.teardown( detach !== false );
  this._fragment = null;

+  window.removeEventListener( 'resize', this._windowResizeHandler );

  this._state = {};
  this._torndown = true;
};

The advantage of doing it as a binding is that it can initialise correctly without you having to declare the values in your data function or manually call the resize handler. At a higher, more philosophical level, the idea is that events are a bad way to model these sorts of values over time — events describe discrete actions, but something like window width is better thought of as a continuous stream of values, and bindings in this context are a way of climbing up that ladder of abstraction in a clean, declarative, zero-cost way. Once you're at that level of abstraction I think it becomes easier to think about certain problems. For example you could reproduce the sort of parallax effects on firewatchgame.com like so:

<:Window bind:scrollY/>

<div class='parallax far' style='transform: translate(0,{{-scrollY * 0.25}})'></div>
<div class='parallax mid' style='transform: translate(0,{{-scrollY * 0.5}})'></div>
<div class='parallax near' style='transform: translate(0,{{-scrollY * 0.75}})'></div>

<div class='content'>
  <!-- scroll at normal speed -->
</div>

<style>
  .parallax {
    position: fixed;
  }
</style>

<script>
  // jk, no JavaScript needed
</script>

If we went down that road, we could easily check that the user wasn't trying to use a non-implemented binding, and I don't think we'd implement stuff that relied on polling.

@PaulBGD
Copy link
Member

PaulBGD commented Mar 16, 2017

So a question about 2 way binding, how would it work with SSR? Would the initial window width/height be 0?

And about using scrollY, how would we handle the scroll event? Updating the entire component tree every time it's called can make scrolling really janky.

@Rich-Harris
Copy link
Member Author

So a question about 2 way binding, how would it work with SSR?

Hmm, great question. I suppose you would kind of have to answer that question anyway if you were using that value — e.g. today you would do something like this...

export default {
  data () {
    return {
      width: typeof window !== 'undefined' ? window.innerWidth : 600,
      height: typeof window !== 'undefined' ? window.innerHeight : 400
    };
  }
};

...in other words you likely have to supply a fallback mechanism whatever it is you're doing. So maybe the same goes for SSR:

const { code, map } = svelte.compile( input, {
  generate: 'ssr',
  fallback: {
    window: {
      innerWidth: 600,
      innerHeight: 400
    }
  }
});

Nice thing about that is the compiler can check that fallback values have been supplied if you're relying on those values in an SSR context, whereas right now if you forgot (i.e. didn't include the typeof window) then you would just get a confusing reference error about window not being defined.

And about using scrollY, how would we handle the scroll event? Updating the entire component tree every time it's called can make scrolling really janky.

It's really no different to how you'd do it any other way — either you add this code...

window.addEventListener( 'scroll', function () {
  component.set({ scrollY: window.scrollY });
});

...or Svelte does. Whether or not it affects the entire component tree is up to you — it only affects the subtree that includes the <:Window> component, and won't trigger updates for any nested components therein that don't use the changed values (because Svelte has a built-in optimisation similar to shouldComponentUpdate). So performance-wise I don't there's any disadvantage to doing it declaratively as opposed to imperatively.

@Ryuno-Ki
Copy link

Hm, weren't there some read-only properties on window or document?

How would work that with (i)frames?

If you're about to implement that, what about adding one for location as well?

@Rich-Harris
Copy link
Member Author

Hm, weren't there some read-only properties on window or document?

Yeah, this is about having an easy declarative way to read those values or respond to events on window or document. Writable values (e.g. document.title) are the special case here.

How would work that with (i)frames?

Not sure what you mean?

If you're about to implement that, what about adding one for location as well?

location doesn't change other than hash, so I'm not sure it warrants its own component (especially since hashchange is an event on window). You can change the URL with the history API, but I think that's too much complex to have a similar declarative representation.

@Ryuno-Ki
Copy link

Have you on your radar to bind to the correct window/document, when executed inside a (i)frame?

@Rich-Harris
Copy link
Member Author

Not in a cross-window sense, no. I think that's a real edge case that probably isn't worth the extra code it would take to support

@PaulBGD
Copy link
Member

PaulBGD commented Mar 16, 2017

window.addEventListener( 'scroll', function () {
  component.set({ scrollY: window.scrollY });
});

Svelte updating can take a while especially if there's a lot of elements, so having it update every time you scroll (which can be called many times a second) would be a really big performance hit.

I suppose in performance specific cases the developer would implement their own scroll listener though..

@Rich-Harris
Copy link
Member Author

Do you have an example of Svelte being slow to update? Something we can adapt for svelte-bench? According to the benchmarks I've looked at so far it's as fast as anything out there.

But yes, if you have a very complex component tree and you bind a lot of things to scrollY, then you will be creating a lot of work. That's true whether you write the code or Svelte writes the code. I don't see what disadvantage <:Window/> introduces in that regard?

@PaulBGD
Copy link
Member

PaulBGD commented Mar 16, 2017

@Rich-Harris it doesn't, I'm just suggesting that there's some edge cases so that we're thorough with a <:Window>/<:Document> implementation. As for svelte being slow, it's not that, it's more so if a component takes a while to render due to its own code then scrolling is janky unless you schedule it later.

@Rich-Harris
Copy link
Member Author

Gotcha. Yes, it certainly warrants a 'use with caution' note when it's documented.

@Ryuno-Ki
Copy link

One way to optimise scrolling it to delay the execution a bit like recommended on MDN

@Rich-Harris
Copy link
Member Author

I did some crude tests to see if scroll events were firing faster than 60fps, and it seemed like they weren't. But my tests weren't exhaustive. If the events really can fire faster than you'd get with rAF-based throttling, and the throttling doesn't introduce any kind of lag, then yeah, I agree that we should do that

@Ryuno-Ki
Copy link

How did you checked that?
I observed, that calling console.log actually distorts the times noticeably..

Could you check with variables, which you do not output, but check in Debugger (against timestamps, maybe?).

@Rich-Harris
Copy link
Member Author

It was pretty crude, I just added a scroll listener that incremented a count, waggled the trackpad aggressively, counted to 10, then checked the count was below 600. My hypothesis was that browsers would throttle the event. Definitely worth some more scientific testing

@PaulBGD
Copy link
Member

PaulBGD commented Mar 20, 2017

Here's a pen: http://codepen.io/PaulBGD/pen/NpXQyO

Edge, Chromium, and Firefox all tested 60/second for me.

@Ryuno-Ki
Copy link

The performance tab of Dev Tools (Firefox) showed an average duration of 0.05s per DOM event for me (you can filter everything out except those to clean the chart).

Thanks for the Pen, @PaulBGD. Have you considered the impact of writing the DOM on the measurement?

@PaulBGD
Copy link
Member

PaulBGD commented Mar 20, 2017

@Ryuno-Ki Well since it's not measuring performance and purely how often the scroll event is called, I don't think writing to the DOM matters.

Rich-Harris added a commit that referenced this issue Mar 26, 2017
Rich-Harris added a commit that referenced this issue Mar 26, 2017
@Rich-Harris
Copy link
Member Author

Thanks for the pen @PaulBGD, that's useful info. Looks like throttling is unnecessary. I've released the first version of <:Window> in 1.13. Still to do:

@ccampbell
Copy link

@Rich-Harris I noticed in the comments here you added the possibility of supporting <:Document> as well. Is that still something you are still planning to add? I have a component where I am doing similar things (attaching events to document in oncreate and removing in ondestroy) so I think that could be useful.

@Rich-Harris
Copy link
Member Author

@ccampbell yes, I think it makes sense to have <:Document> as well, though there isn't a separate issue for it yet — would like to be able to bind to visibilityState for example, or set the title. What events did you have in mind? (It would support all document events anyway, just curious about use cases)

@ccampbell
Copy link

So my specific example is I have a component that is draggable around the window. You could almost handle that all at the component level, but what I tend to prefer doing is binding the mouseup and mousemove events to the document and keeping mousedown on the element.

There are cases when you are dragging something where the mouse could move outside of the bounds of the element (think about a video scrubber for example). So this way the dragging can continue even when you are not within the element being dragged anymore.

I’m sure there are other (perhaps better) ways of doing what I’m doing, but I think it is a valid use case 😄

@bestguy
Copy link

bestguy commented Mar 31, 2017

Minor comment but the colon prefix on: <:Window /> looks 'weird' and concerned it might add confusion. Is just <Window /> an option?

@Rich-Harris
Copy link
Member Author

@ccampbell the way I've done that in the past is with a custom drag event: https://svelte.technology/repl?version=1.13.2&gist=114c93db7ff6131a64eaa2da239be9a4. But yeah, I reckon there probably are cases where you could use <:Window> for that.

@bestguy I actually think the exact opposite is true — making it look like a normal component would be highly confusing. This way, you see straight away that it's something else (and if you're not already familiar with it, you're likely to go and read about it in the documentation, where we'll eventually add a section on meta-tags!) and you instinctively understand that it might not behave exactly like other components (for example, the fact that bind:innerWidth is a read-only binding).

Less important, but it also makes validation much easier — we can determine at parse time, without having to wait until we've statically analysed your JavaScript, that this is a meta-tag that needs to obey certain rules (e.g. it has to be at the top level of the component template).

@bestguy
Copy link

bestguy commented Mar 31, 2017

That makes sense, thanks for the reasoning. I suppose the crux of it was the look of the colon prefix itself but minor, thanks.

@arxpoetica
Copy link
Member

THIS IS SO USEFUL AND GENIUS!!! Can't tell you how many times I needed something like this when using Ractive, thank you new & awesome Svelte.

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

7 participants