Allow String objects as source

[piranna]

At

xmldom/lib/dom-parser.js

Line 95 in 8116f56

if (source && typeof source === 'string') {

is being checked that source is a string literal, but the spec dictates is a DOMString object, that in Javascript, it maps directly to the corresponding String class. So, String objects, should be allowed, but instead they are directly rejected.

Here we have two options, call to .toString() to all values provided in the source argument, or check that source is a string literal or a String object.

[karfau]

I double checked and we are not talking about strings created using the String function, but by using the String constructor like this:

var S = new String('hello');

Just for later reference:

• typeof S // object
• S instanceof String //true
• S.valueOf() // 'hello' (primitive)
• String(S) // 'hello' (primitive)

After reading through the specs a bit more I have my doubts.
Here is what MDN says:

A DOMString is a sequence of 16-bit unsigned integers, typically interpreted as UTF-16 code units. This corresponds exactly to the JavaScript primitive String type.

✔️

When a DOMString is provided to JavaScript, it maps directly to the corresponding String.

❓ What does "is provided to Javascript" mean? There seems to be some transition from values outside of javascript?

When a Web API accepts a DOMString, the value provided will be stringified, using the ToString abstract operation. (For types other than Symbol, this has the same behavior as the String() function.)

If I understand this correctly any API accepting DOMString in the spec should accept all the possible values listed in the ToString specification.
Which we could implement using String for all input other than Symbols.
If we take this seriously it will affect the whole exposed API, not just new DOMParser().parseFromString().
It would be an API much more similar to "native automatic value conversion" in non strict javascript, since we would basically allow "any" input value. There are even specific APIs that convert null to an empty string.

I also checked what browsers are doing:

So there is even a different in handling XML and HTML...

Even the types typescript provides for the same API you reference only allow the primitive string type.

interface DOMParser {
    parseFromString(str: string, type: SupportedType): Document;
}

I must say that I personally prefer a more clearly typed API that only allows strings as input (explicitly stating when it allows null or undefined).

With to goal to still allow users to use our lib as described in the spec, it could make sense to implement and expose a IDLToString method, but I would not want to change our whole API to implicitly always call it.

@piranna What do you think?

[piranna]

Good analisys, it made me doubt and double check myself too :-)

A DOMString is a sequence of 16-bit unsigned integers, typically interpreted as UTF-16 code units. This corresponds exactly to the JavaScript primitive String type.

When a DOMString is provided to JavaScript, it maps directly to the corresponding String.

I find this two quotes from the docs conflicting, since there's no String primitive, but instead string one, String is an object class. Can you point me to the first one so we can propose to fix the typo?

What does "is provided to Javascript" mean? There seems to be some transition from values outside of javascript?

That's my interpretation. MDN documentation is confusing on this point, but seems that when a DOMString is provided to Javascript from the platform (C++, Rust...), they are directly mapped to Javscript String objects, not string literals.

If I understand this correctly any API accepting DOMString in the spec should accept all the possible values listed in the ToString specification.

Yes... but it's as simple as calling to .toString() method on the input data to sanitize it, that's what I'm proposing on this issue :-) This would also works for Symbols, so there's no need to re-implement the ToString protocol as you propose.

It would be an API much more similar to "native automatic value conversion" in non strict javascript, since we would basically allow "any" input value.

Yes, maybe... but seems it's how the spec dictates it must work (at least in Chrome):

I must say that I personally prefer a more clearly typed API that only allows strings as input (explicitly stating when it allows null or undefined).

With to goal to still allow users to use our lib as described in the spec, it could make sense to implement and expose a IDLToString method, but I would not want to change our whole API to implicitly always call it.

@piranna What do you think?

My opinion is to stick to the spec and not add non-standard APIs or methods. That means:

1. change the Typescript types to allow any as spec behaviour works, or left them as string as they currently are to provide a help to Typescript only developers
2. calls to .toString() method internally on all input data to sanitize it, as it seems the Javascript spec works

[karfau]

I will have to think about it.
Still not convinced that we should allow any as input for all those APIs and call .toString() on all the input.

[piranna]

Still not convinced that we should allow any as input for all those APIs and call .toString() on all the input.

As I said, the other alternative is to have it typed as string, since this will be endorsed only by Typescript, and call .toString() so it works properly with vanilla Javascript.