Dom API Create throws "Illegal constructor"

[olivercoad]

Calling HTMLImageElement.Create (512., 512.) throws "Uncaught TypeError: Illegal constructor".

I came across the same issue as #25 but the fix for that didn't work.
I was going to just submit a PR with the fix @alfonsogarciacaro proposed because I can confirm that changing

[<Emit("new $0($1...)">]

to

[<Emit("new Image($1...)")>]

works.

However I noticed that this pattern is used throughout Browser.Dom.fs so I tried ___.Create for a few other elements types and it looks like they all have the same issue (atleast all the ones I tested).
It's not like it's just a few types that weren't initially tested and happen to have quirks; has something changed about the way fable handles type names or something?

[alfonsogarciacaro]

Strange. No, there has been no change in the resolution of imported types. I just realized we haven't yet properly fixed #25, I'll do that. But this is a special case, because the name of the constructor and the type are different. Can you please share code of similar fails with other types?

[alfonsogarciacaro]

Released 2.1.1 with a fix for Image. 82d359d

[olivercoad]

Thanks for the fix!

I decided to just test everything in Browser.Dom.Api.fs (thanks vscode for multiline edit!).
The only ones that didn't have an Illegal contstructor TypeError or an is not defined ReferenceError are

• Document
• DocumentFragment
• HTMLImageElement
• ImageData
• Range
• Worker
• NodeFilter (doesn't have a Create method)

I don't know enough about DOM to know if half of the others even make sense to be creating like this but I'd guess many of these probably shouldn't even have a Create method or should use document.createElement instead of the constructor. This stackoverflow answer gives some insight into why typescript has a constructor defined for DOM elements https://stackoverflow.com/a/26221525

Tested using Fable.Browser.Dom v2.1.1 in chrome 85.0.4183.121

TestFableBrowser.fs.txt
fable-browser issue 47 test results.txt

[alfonsogarciacaro]

Thanks a lot for the detailed report @olivercoad! You're definitely right we shouldn't expose the constructor for these elements. Would it be possible to send a PR to remove/comment the Create method for these types? Or change the emitted JS to document.createElement, whatever you think is the best approach.

[olivercoad]

Yep. I've made a start with changing to document.createElement.
It will take a while and it turns out there are actually quite a lot of cases where it's created slightly differently.

For the onces where it doesn't make sense to have a Create method (such as DocumentTypeType), would you prefer to use interface end or to just remove the type?

I'm using Fable.Mocha to make it quicker to test so could include the test project in the PR if you'd like.

[alfonsogarciacaro]

I'm really sorry @olivercoad, I was just trying to clean notifications in Github and realized I completely missed this one! 🤦 Not sure about removing interfaces or leaving them empty, but if they're not used anywhere it may make sense to remove them.

It'd be very nice to add tests to this repo so if you can include them in your PR it'd be fantastic!