Incorporate missing facades used by Indigo
Opened this issue · 11 comments
This is a slightly lazy issue, apologies for that, but I caught the new maintainer announcement and thought I'd get in quick! :-)
Indigo uses Scala-js-dom a lot as you can imagine, but in a very few specific cases I've had to write my own facades or extensions to existing facades where functionality is missing, particularly for WebGL 2.0 support.
They all live here, bear in mind I don't believe WeakMap is in use, and they aren't supposed to be complete interfaces they only add what I need, but if it helps:
https://github.com/PurpleKingdomGames/indigo/tree/master/indigo/indigo/src/main/scala/indigo/facades
I don't know how much time I have but I could try and wrap some of these up into a PR if it helps.
WeakMap is standardized in ECMAScript, so it should be added directly in the Scala.js standard library, instead of here.
Thanks for this! If you have a chance, would be awesome if you can make a PR even if it's only for a few of the missing things, anything helps :)
Hi, I came back to have a look at this.
The interfaces I've got (and I haven't checked if they've been added since I wrote them yet) are:
FontFaceTextMetricsFullScreenElementIndigoCanvasRenderingContext2Dwhich just addsvar direction: String = js.nativetoCanvasRenderingContext2DWebGL2RenderingContextwhich extendsWebGLRenderingContext
The last one is the biggest, but all of them are partials that just add the features I needed, and are not scaladoc'd yet (but I can have a go).
It looks there are two groups of facades to move over, the first four small an uncontentious ones that I may do as a separate PR each (assuming I find they are still missing).
The more dubious one I was going to move over was the WebGL 2.0 stuff. I had a quick scout around the project though and noticed the WebGL 1.0 discussion:
Technically Safari hasn't adopted WebGL 2.0 yet - it is imminent I believe (https://caniuse.com/?search=webgl%202) - but they are way behind. Should the WebGL 2.0 interfaces be in the experimental area do you think?
Went in to add the following to CanvasRenderingContext2D :
/**
* Possible values: ltr, rtl, inherit
*
* MDN
*/
var direction: String = js.native
But according to MDN this is experimental too.
What do we do in this case?
@davesmith00000 Thanks for looking into this!
It looks there are two groups of facades to move over, the first four small an uncontentious ones that I may do as a separate PR each (assuming I find they are still missing).
That would be awesome, thanks!!
Regarding experimental stuff, there's also the separate issue of how well-supported it is across browsers. E.g., an API could be experimental but still supported by all major browsers. On the other hand, it might not be experimental but that doesn't matter if browsers don't support it 🙃 In any case should still have a formal strategy how to determine these cases going forward, cc @japgolly.
Looking specifically at CanvasRenderingContext2D.direction, it falls into the unfortunate category of being both experimental and also completely lacking support in Firefox.
Ok, so philosophically the question is:
Does this library represent the full specification and require the developers to know what they can and can't use (which is the case if you are a normal JS developer), or only a subset of the features that are known to be broadly supported?
Which I believe is related to what #514 is about. I'm completely fine with either, seems like you're suggesting it's a subset? I think that's the more difficult path to follow.
Personally, I'd expected the library to attempt to represent the whole specification without fear or favour, but maybe that's just too much work or undesirable for some reason? Nonetheless, if thats the case then the ones to add from my list are (I think - assuming we don't care about IE?):
FontFaceTextMetricsFullScreenElementWebGL2RenderingContext
I don't believe any of that is experimental, but then again, Safari won't do WebGL 2 and support for TextMetrics is patchy... Bah! 😄
Personally, I'd expected the library to attempt to represent the whole specification without fear or favour, but maybe that's just too much work or undesirable for some reason?
Personally I'd lean this way myself, especially via a scheme like #487. But historically speaking, I'm not 100% what this library's mission is. The use of an experimental package and frequent references to www.caniuse.com in PRs suggests that it is not that.
I would suggest leaving experimental in the past, and represent instead the full spec, independently of browser support. Otherwise, there's never a good time nor a good way to have things "graduate" out of experimental anyway.
Ok so... I've read the updates on issues #514 and #487 there's a bit of a dependancy chain here I think.
If you go ahead and do code generation then this ticket (and probably many others...?) can be closed as redundant.
Assuming you don't do that (yet?), in order to do this ticket we'd ideally need #370 closed/merged and the experimental package deprecated/merge to avoid confusion.
Then I could merge over the missing stuff, and then you'll need to draft some exciting comms for the next round of release notes explaining all this. 😄
I'll hold off until I hear back. Equally happy for someone else to beat me to it. 😉
@davesmith00000 thanks for following along!
At the moment, a code generation scheme is still a bit mythical, so I wouldn't worry about that 😆 (although, if you feel inclined please add a comment showing your support in that discussion, if we can get a signal that the community is happy with the positives/negatives of an auto-generated codebase that is extremely helpful).
But you are right, I think the next big step in scala-js-dom is cleanups etc. occurring in #458 and possibly some follow-up PRs. However, as long as you follow the (currently draft) contributing guidelines proposed in #523 I think we should probably be able to merge in your work fairly easily despite all the shuffling around. Could be wrong though so maybe the safer strategy is to wait 😅