BlobPropertyBag needs a constructor about scala-js-dom HOT 11 CLOSED

You're correct- BlobPropertyBag doesn't have an underlying type, but is in fact just a regular object, so this definition is not much use.

http://www.w3.org/TR/2012/WD-FileAPI-20121025/#dfn-BlobPropertyBag

I'll try to address this tomorrow unless you fancy a stab at it, should be a simple enough fix.

from scala-js-dom.

I've got other things on my plate (updating some libraries for 0.6.3), and I don't actually use this myself, so I'd just as soon you did it -- I was just recording it while I thought of it, since it came up on the list. Thanks...

from scala-js-dom.

Alright, no worries. I'll PR it tomorrow, then.

from scala-js-dom.

@lihaoyi @sjrd

Quite often we have IDL specifications that define structural types for a simple js object passed to a constructor or function. I've currently taken to declaring these as js.Dynamic, following others. Making this trait a class and adding a constructor as suggested will fail, since scala-js will assume that BlobPropertyBag is actually defined in the global js namespace.

It's a bit of a shame having a js.Dynamic parameter though- because then we all have to go over to the w3c docs to figure out what on earth it expects. :(

So... should I just declare this parameter to Blob as js.Dynamic and be done with it (which at least removes the confusion here), or is there a better way to handle this common scenario in the library? I'm not averse to doing a bit of a clean-up for 0.9.0 if you think we can improve overall here.

from scala-js-dom.

I'd actually been thinking of the "constructor" being an apply() method in a companion object. Don't recall offhand whether that's legal with a js.Object trait...

from scala-js-dom.

Typing it like it's done now is good. What it needs is a way to construct it easily and type-safely. As you said, it can't be an actual constructor, but it can be an apply on the companion object:

object BlobPropertyBag {
  def apply(`type`: String): BlobPropertyBag = {
    js.Dynamic.literal(
        `type` = `type`
    ).asInstanceOf[BlobPropertyBag]
  }
}

from scala-js-dom.

Great! yes, this feels a lot better. I'll take that approach for here on in. I can think of a couple of other places where this would help, too, although perhaps strengthening js.Dynamic to more specific traits elsewhere in the library would break existing code. I'll address this specific issue tomorrow and queue up a separate PR for any sensible js.Dynamic signatures I find so it can be merged whenever.

Thanks for the feedback!

from scala-js-dom.

By and large, this is the way to go for any of these "options" objects so long as their parameters don't have polymorphic types. (If they do have a bunch of polymorphic parameters, this starts to break down -- at that point I recommend JSOptionBuilder, but we don't want to do that in a core library like this.)

That said, all of the params should usually be optional if possible. Will everything work as desired if the parameter is declared as UndefOr[String] = js.undefined? In particular, are there implicits in place so that passing in a String will do the right thing? (I think so, but I haven't tried it, since I had to go the JSOptionBuilder route early.)

from scala-js-dom.

Yes, js.UndefOr[String] = js.undefined will do the right thing. Almost: it will actually create a field whose value is undefined, which is different than the field not existing at all in theory (although in practice it rarely is).

from scala-js-dom.

Yeah, I tend to assume that APIs will be well-behaved in taking undefined. So far, I don't think I've come across any exceptions...

from scala-js-dom.

If that's not the case, here is a version that behaves properly:

@inline
def apply(`type`: js.UndefOr[String] = js.undefined): BlobPropertyBag = {
  val result = js.Dynamic.literal()
  `type`.foreach(result.`type`= _)
  result.asInstanceOf[BlobPropertyBag]
}

from scala-js-dom.