gopherjs / websocket

websocket provides high- and low-level bindings for the browser's WebSocket API.
BSD 3-Clause "New" or "Revised" License
111 stars 25 forks source link

Allow for SockJS; removed dom import to reduce file sizes; made WebSocket an interface #4

Closed liamcurry closed 7 years ago

liamcurry commented 9 years ago

This merge adds/changes a few things:

dmitshur commented 9 years ago

Hi, thanks for the PR, but you should be aware of #3 where we're working on a more complete version of this package. There are some good ideas in this PR we can adapt into there. /cc @nightexcessive

dmitshur commented 9 years ago

Added websocket.NewWithGlobal function to allow specifying the variable on the window.

Can you give an example of a situation where this would be used? Edit: I see you provided SockJS as an example of that.

dmitshur commented 9 years ago

3 has been merged, so we can start thinking about applying positive changes from here to close this.

dmitshur commented 9 years ago

I'd like to look into this. I am wondering if the dead code elimination is working as much as I expect it to. I've noticed (like you) that importing the dom package and using only a very small subset of its types/methods results in a huge increase in generated file size. I am wondering DCE can be more aggressive at eliminating unused things.

Note that importing dom only increases file size if it's not already imported in the user code. So while avoiding it in websocket bindings may be feasible, that may not work well in more complex user programs.

Anyway, I will look into it/think about it.

dominikh commented 9 years ago

DCE isn't really to blame here. Almost all types in the dom package are alive, because they can be created dynamically at runtime (there is one type per HTML element.)

The majority of the file size right now is caused by GopherJS encoding method sets directly in the file, without any deduplication. dom makes heavy use of embedding, of rather large interfaces, which means that we have a ton of types with a ton of methods. In one of my projects, I have a 2.3 MB (not minified) JS file. Of that, 1.3 MB are just the method sets. Deduplicating these naively, (i.e. sort -u) brings it down to 284 KB, and we aren't even considering partial duplication yet (e.g. [foo, bar, baz] and [foo, bar, qux])

I told Richard about this and we'll see if/how it can get optimized :-)

dmitshur commented 9 years ago

DCE isn't really to blame here. Almost all types in the dom package are alive, because they can be created dynamically at runtime (there is one type per HTML element.)

Thank you for that insight. I suspected there was a reason why those unused types could not be eliminated, but wasn't sure why.

By "created dynamically", do you mean using strings and reflect package? Otherwise if my code never mentions the types, it can't be created, right?

It's good to hear there is still opportunity to reduce the size of generated code through work on methods sets.

dominikh commented 9 years ago

document.GetElementByID("foo") -- that might be an HTMLDivElement or an HTMLInputElement or any other *Element type.

dmitshur commented 9 years ago

I see. What if I don't use GetElementByID?

dominikh commented 9 years ago

Any function or method that returns a value of type Node, Element or HTMLElement or a struct type that includes one of these will go through the wrappers. That includes any event-related code, because the event target is part of the event.

dmitshur commented 7 years ago

I'm going to close this PR because it's inactive and too out of date to be actionable.

If there's anything else that needs to be done, let's discuss that in a new issue and PR, we can reference this one as needed. Thanks for contributing.