Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More properties for the browsers compatibility #409

Open
AlttiRi opened this issue May 7, 2022 · 5 comments
Open

More properties for the browsers compatibility #409

AlttiRi opened this issue May 7, 2022 · 5 comments
Labels
needs investigation Information is missing and needs to be researched
Milestone

Comments

@AlttiRi
Copy link

AlttiRi commented May 7, 2022

I would like to have access to more properties like it is in the browsers to write a browser compatible code.

For example, innerHTML, dataset, body, head, href, src, classList:

console.log(doc.querySelector(".foo").textContent);  // works
console.log(doc.querySelector("a").getAttribute("href")); // works

// the follow props are undefined in xmldom
console.log(doc.querySelector(".foo").classList);
console.log(doc.querySelector("a").innerHTML);
console.log(doc.querySelector("a").dataset?.xxx);
console.log(doc.querySelector("img").src);
console.log(doc.body);
console.log(doc.head);
console.log(doc.head?.innerHTML);
console.log(doc.location); // should be null

// the questionable case:
console.log(doc.querySelector("a").href); 
// (in a browser it returns a resolved URL to global `location`, for example "https://github.com/#"
// jsdom resolves it relative to `about:blank`)

The full code:

import selectModule from "query-selector";
import {DOMParser as DOMParserSimple} from "@xmldom/xmldom";
function getDOMParser() {
    if (globalThis.DOMParser) {
        return globalThis.DOMParser;
    }
    const select = selectModule.default;

    const doc = new DOMParserSimple().parseFromString("<dummy/>", "text/xml");
    const documentPrototype = Object.getPrototypeOf(doc);
    documentPrototype.querySelectorAll = function querySelectorAll(selector) {
        return select(selector, this);
    };
    documentPrototype.querySelector = function querySelectorAll(selector) {
        return select(selector, this)[0];
    };
    return DOMParserSimple;
}
const DOMParser = getDOMParser();

// ---

let html = `
<style>a {color: red};</style>
<div style="display: none">qwe</div>
<div class="foo">asd <a href="#" data-xxx="XXX">[link]</a></div>
<img src="https://example.com/1.png" alt="1"/>`;
let doc = new DOMParser().parseFromString(html, "text/html");

console.log(doc.querySelector(".foo").textContent);  // works
console.log(doc.querySelector("a").getAttribute("href")); // works

// the follow props are undefined in xmldom
console.log(doc.querySelector(".foo").classList);
console.log(doc.querySelector("a").innerHTML);
console.log(doc.querySelector("a").dataset?.xxx);
console.log(doc.querySelector("img").src);
console.log(doc.body);
console.log(doc.head);
console.log(doc.head?.innerHTML);
console.log(doc.location); // should be null
console.log(doc.querySelector("a").href); // (in a browser it returns a resolved URL, jsdom returns `about:blank#`)

#92 (comment)

@karfau
Copy link
Member

karfau commented May 7, 2022

Just because most of your post seems to be related to adding querySelector and querySelectorAll, I'm assuming it is important for you to to not only have the additional properties, but also those methods.

I would just like to highlight that we can continue to discuss adding the properties to Document and or Element/Node as they are specified.
But any discussion regarding the querySelector* methods should stay in #92

@karfau karfau added this to the before 1.0.0 milestone Oct 19, 2022
@karfau karfau added the needs investigation Information is missing and needs to be researched label Oct 19, 2022
@karfau
Copy link
Member

karfau commented Oct 19, 2022

We will have to check in which specifications those things are defined.
I think most of those props only exist in HTML documents.
We will not be able to support .href since it relies on the document location, which doesn't exist in all environments (e.g. Node) that xmldom supports.

@AlttiRi
Copy link
Author

AlttiRi commented Oct 19, 2022

It's possible just to add one optional parameter to the constructor — current location to resolve href relatively to it.
I think href is one of the frequently used keys while working with HTML by JS.

@karfau
Copy link
Member

karfau commented Oct 19, 2022

Trying to better understand your use case:
In the example code you are ponyfilling the browser based DOMParser with the one from xmldom, to be able to use the code in other runtimes, right?

But you are always using the constructor without any arguments. Since the browser native DOMParser doesn't support constructor arguments, would you just always pass the argument?

In xmldom the a tag element doesn't have any special behavior, so joining the value from the href attribute with whatever the document.location is, would mean we have to add special attribute behavior.

For now I would still suggest to handle the joining on the level of your code that handles ponyfilling: if the href attribute value starts with #, join it with whatever you think the location should be.

Or start looking into the code to add a PR for that feature.
But we will not have much capacity to support it beside code reviewing.

@AlttiRi
Copy link
Author

AlttiRi commented Oct 19, 2022

the browser native DOMParser doesn't support constructor arguments

It just ignores them. So, I think it's safely to extend the constructor. The code will be compatible with the browsers.

new DOMParser({location: "https://example.com"});

location is an example name here.

For example, from the my code above: console.log(doc.location); prints null in a browser. While it correctly resolves a relative href based on the globalThis.location.


Well, here is how I can manually resolve it with URL:

const resolveLocation = "https://example.com";
new URL(doc.querySelector("a").getAttribute("href"), resolveLocation).href

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation Information is missing and needs to be researched
Projects
None yet
Development

No branches or pull requests

2 participants