xmldom / xmldom

A pure JavaScript W3C standard-based (XML DOM Level 2 Core) `DOMParser` and `XMLSerializer` module.
MIT License
364 stars 88 forks source link

Can xmldom support querySelectorAll? #92

Closed masx200 closed 3 years ago

masx200 commented 4 years ago

support querySelectorAll

TypeError: document.querySelectorAll is not a function

masx200 commented 4 years ago

https://www.npmjs.com/package/jsdom

Use jsdom to solve the problem

karfau commented 4 years ago

In the current state of this library, I would consider it out of reach, potentially even for v1.0 since

Maybe it can be achieved by adding some dependency, but this also comes with an additional maintenance cost, since it currently doesn't have production dependencies.

So I think going for something like jsdom for that is a valid approach.

This is of course just my personal opinion.

silverwind commented 4 years ago

I'd say the project should stay zero-dependencies. I'd be happy if if there were a getElementsByClassName which should be sufficient for many use cases of querySelectorAll.

morrisallison commented 4 years ago

For those who can't use JSDOM, I was able to get my use case working using the query-selector package. Your mileage may vary.

import querySelector from "query-selector";
import { DOMParser } from "xmldom";

const doc = new DOMParser().parseFromString("<foo />", "text/xml");
const documentPrototype = Object.getPrototypeOf(doc);

documentPrototype.querySelectorAll = function querySelectorAll(selector) {
  return querySelector(selector, this);
};
karfau commented 4 years ago

I think this issue could already be closed. We could of course add some lines in the docs (or in some example section?) to make it easier to find it, but the conclusion is that xmldom will not implement querySelectorAll itself any time soon.

What do you think @brodybits ?

brodycj commented 3 years ago

It would be awesome if we could get this documented.

@karfau I may need a few days to get to the other recent threads on xmldom. Please remind me in case I forget.

karfau commented 3 years ago

I think it's a good idea to document this decision. It's also related to #176

karfau commented 3 years ago

Another round of trying to argue for closing this issue:

  1. It's currently not clear where to document this. (The readme should imo contain less documentation)
  2. The suggested workaround using query-selector is most likely not the only option. There has not been any update to that library in 2 years, which could mean it just works and is stable or it means it became inactive. Do we want to invest time to figure out the details and maintaining a recommendation?
  3. When people search this repo for querySelectorAll they will find this issue (even if it has been closed) including the work around(s) and be able to judge themselves.

Feel free to reopen, but please provide some arguments/proposals of how to solve the above points.

atava commented 3 years ago

I feel like posting my use case.

I am using the cross-fetch library to implement isomorphic fetch in my app (i.e. relying on the browser fetch when running in the browser, using other code if running on node), and would have liked to proceed in the same way with regard to DOMParser. This library seemed to provide that up to the point of the querySelector methods, which are absent.

My app should be able to run the same code both in the browser and outside of it, hence my requirements.

Thanks anyway for clarifying this and for this library.

karfau commented 3 years ago

@atava thx for adding your use case.

There is thing I want to point out/clarify: The code you write, that uses fetch is the same for node and browser, but using cross-fetch means that the implementation of fetch is not the same depending on the runtime (even though the code for the node version might be shipped when bundling).

I think it should be technically possible to create a cross-dom wrapper that would use xmldom and some other library for adding querySelector/querySelectorAll methods to the prototype and just rely on the native browser implementation when it is available.

But this is not something that the xmldom maintainers will do, since right now xmldom has quite some differences in behavior compared to the implementation of browsers, and with the current amount of resources it will take quite a while to align it, so it's most likely not a good idea.

So using the jsdom library to polyfill the native API when running your code with node, is the better option currently.

atava commented 2 years ago

@karfau Thanks, and sorry for replying so late.

To further elaborate on this, I would need an isomorphic fetch as well as isomorphic querySelector and querySelectorAll methods for my app as it does some Web site scraping and ideally it would need:

Thanks for clarifying that this library could not and would not add a 1:1 implementation of querySelector & co.

karfau commented 2 years ago

Just found out about https://github.com/fb55/css-what which might be helpful if somebody wants to implement a node visitor based on it (outside of this repository).

AlttiRi commented 2 years ago

It would be great if DOMParser work same way as in modern browsers.

If parseFromString would return Document with body, head with textContent, innerHTML props querySelector, querySelectorAll methods.

In order to have the same code for Node.js and browsers.


Some examples:

let html = `
<style>a {color: red};</style>
<div style="display: none">qwe</div>
<div class="foo">asd <a href="#">[link]</a></div>
</div>`;
let doc = new DOMParser().parseFromString(html, "text/html");

doc.body.textContent:

"qwe
asd [link]
"

doc.body.innerHTML:

"<div style="display: none">qwe</div>
<div class="foo">asd <a href="#">[link]</a></div>
"

doc.querySelector("a").textContent:

"[link]"
AlttiRi commented 2 years ago

jsdom' DOMParser works fully as the browser's DOMParser. But using jsdom just only for DOMParser is overkill, it's 10 MB of code, memory and computation wasting for unused functional.

Also they are not interested in this issue. https://github.com/jsdom/jsdom/issues/3364

karfau commented 2 years ago

There will not be any support for querySelector or querySelectorAll in this package without a major increase in maintainer capacity. Which is the same reason why we can not possible have a goal to be feature complete with modern browsers.

And people already suggested above how this feature can be "added" on the fly using an external dependency. As long as this package doesn't have any external dependencies, we will not add one just to support those functions.

But for adding body and/or some of it's properties, feel free to create a PR or create a new issue to discuss the details.