search

dennwc / dom

DOM library for Go and WASM
Apache License 2.0
486 stars 59 forks source link

Change method signature to js.Class(class string, path ...string) #40

Closed smoyer64 closed 5 years ago

smoyer64 commented 5 years ago

Both JavaScript and TypeScript allow access to global objects via dotted notation. At the same time, many libraries wrap their objects, classes and methods to avoid adding them to the global namespace.

Allowing dotted notation in js.Get() would simplify accessing wrapped types and would automatically supplement both js.Class() and js.New(). The following code works:

mdc := js.Get("mdc")
chips := mdc.Get("chips")
chipset := chips.Get("MDCChipSet")
chipsetEl := dom.Doc.CreateElement("div")
chipsetEl.SetAttribute("class", "mdc-chip-set")
chipset.Call("attachTo", chipsetEl)

But this code is much simpler to write and has the added benefit of using the js.Class() caching:

chipset := chips.Class("mdc.chips.MDCChipSet")
chipsetEl := dom.Doc.CreateElement("div")
chipsetEl.SetAttribute("class", "mdc-chip-set")
chipset.Call("attachTo", chipsetEl)
smoyer64 commented 5 years ago

Looking into this a bit more, I see that the Get() method allows zero or more path sections. This allows the code above to be simplified as follows:

chipset := chips.Get("MDCChipSet", "mdc", "chips")
chipsetEl := dom.Doc.CreateElement("div")
chipsetEl.SetAttribute("class", "mdc-chip-set")
chipset.Call("attachTo", chipsetEl)

I'd suggest that the signature of the Class() method be changed to:

func Class(class string, path ...string) Value {

Unfortunately, this can't be extended to the New() method as both path and args would need to be variadic. As the code is currently written, you can use class caching for classes that are global. Another option might be to have a method with the signature:

func (v Value) Class(class string) Value {

It's a shame that the underlying syscall/js code doesn't support this.

dennwc commented 5 years ago

Yeah, extending the Class makes a total sense. Probably it won't be a problem with New since you can always do things like Class("foo", "Bar").New(). So feel free to submit a pull request. Otherwise, I'll implement it at the end of the week.

smoyer64 commented 5 years ago

PR #41 submitted - my resulting code is much nicer and takes advantage of class caching!

chipsetClass := chips.Class("MDCChipSet", "mdc", "chips")
chipsetEl := dom.Doc.CreateElement("div")
chipsetEl.SetAttribute("class", "mdc-chip-set")
chipset := chipsetClass.Call("attachTo", chipsetEl)
dennwc commented 5 years ago

Thanks :) Also note that you don't need to call JSValue at the end.

smoyer64 commented 5 years ago

I had a few places where it didn't compile unless I added JSValue ... I'm still working on why but it's good to know that it shouldn't be required.