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

Expose DOM Level 2 Elements #40

Closed
esbullington opened this issue Mar 16, 2020 · 10 comments · Fixed by #637
Closed

Expose DOM Level 2 Elements #40

esbullington opened this issue Mar 16, 2020 · 10 comments · Fixed by #637
Labels
documentation Improvements or additions to documentation help-wanted External contributions welcome spec:DOM-Level-2 https://www.w3.org/TR/DOM-Level-2-Core/
Milestone

Comments

@esbullington
Copy link

esbullington commented Mar 16, 2020

Glad you all have forked and are maintaining xmldom, thank you!

There were several issues in the old unmaintained repo dealing with exposing DOM Level 2 object interfaces, such as Node, Element, NodeLike. Right now, those classes are private.

There are multiple library that deal with the browser DOMParser that make use of these for things like instanceof checks. As one example, I'm using bs-xml, which wraps DOMParser for Bucklescript/ReasonML, and which has to check if an object is a Node or Element.

Would you be open to a very simple PR that exposes those classes? Specifically, the ones listed as fundamental interfaces in the Level 2 DOM Core spec?

DOMException
ExceptionCode
DOMImplementation
DocumentFragment
Document
Node
NodeList
NamedNodeMap
CharacterData
Attr
Element
Text
@brodybits brodybits added enhancement help-wanted External contributions welcome labels Mar 16, 2020
@esbullington
Copy link
Author

esbullington commented Mar 16, 2020

To clarify my issue, I can submit a PR, just wanted to make sure you're open to a PR for this issue. Looks like yes based on the labels. Thanks!

@brodybits brodybits added this to the 0.5.0 milestone Mar 16, 2020
@brodybits
Copy link
Member

Thanks, yes. Added to intermediate milestone before 1.0.0 for now (see milestones here). Unfortunately I do not have much time right now to review and merge contributions right now, hope another maintainer will have a chance.

esbullington pushed a commit to esbullington/xmldom that referenced this issue Mar 16, 2020
This was referenced Jun 21, 2020
@cortopy
Copy link

cortopy commented Dec 3, 2020

I'm also caught by this issue. My workaround is to rely on typescript types for the dom:

function isAttr(attr: any): attr is Attr {
  return attr.constructor.name === 'Attr';
}

@cortopy
Copy link

cortopy commented Dec 9, 2020

Actually, ignore my workaround. I was just hit by a bug in production code, as minified code will change the constructor's name (it's a private class after all) and the check won't work

@karfau karfau added documentation Improvements or additions to documentation spec:DOM-Level-2 https://www.w3.org/TR/DOM-Level-2-Core/ labels Jan 21, 2021
@karfau karfau modified the milestones: 0.5.0, 0.6.0 Jan 21, 2021
@karfau
Copy link
Member

karfau commented Apr 17, 2021

Sorry for joining the conversation so late, but there is one issue that we need to be aware of:
The DOM spec(s) only define interfaces for those things, but we can only expose the classes.
So it means that by exposing them we "risk" that somebody starts using the constructors of those classes.

If somebody knows how to "mark the constructors as private" or how other projects do that, please let us know.
I could imagine to only expose them in a "namespace"which could also be the DOMImplementation, since it's meant to be used to actually create instances of Documents "manually".

So we would only export DOMImplementation and it would provide access to all the "interfaces" so you could say:

const doc = new DOMParser().parse('<xml/>');
if (doc instanceof DOMImplementaiton.Document) { // true

If DOMImplementaiton is "to long/unpractical" we could export a (read only) object called DOM,
which provides the same access, so it would just be

if (doc instanceof DOM.Document) {

Since we are planning to move the types definition into the repo, based on the actual doc comments and types present in this code base (see #191), we might also be able to mark the constructors as @private and "only export the interface" this way...

What do you think?

@karfau karfau added awaiting response Maintainers are waiting for information question Contains open questions that need to be answered and removed help-wanted External contributions welcome enhancement labels Aug 21, 2021
@karfau karfau modified the milestone: before 1.0.0 Dec 21, 2021
@danmichaelo
Copy link

danmichaelo commented Mar 21, 2023

...

So we would only export DOMImplementation and it would provide access to all the "interfaces" so you could say:

const doc = new DOMParser().parse('<xml/>');
if (doc instanceof DOMImplementaiton.Document) { // true

If DOMImplementaiton is "to long/unpractical" we could export a (read only) object called DOM, which provides the same access, so it would just be

if (doc instanceof DOM.Document) {

Since we are planning to move the types definition into the repo, based on the actual doc comments and types present in this code base (see #191), we might also be able to mark the constructors as @private and "only export the interface" this way...

What do you think?

That would be lovely! I just found myself with the same need. I think both would work fine and don't have a strong preference – a longer name like DOMImplementation is fine if you want to indicate that this is something most users won't need, but DOM is also very neat.

@karfau karfau added help-wanted External contributions welcome and removed awaiting response Maintainers are waiting for information question Contains open questions that need to be answered labels Mar 21, 2023
@karfau
Copy link
Member

karfau commented Mar 21, 2023

The priority for this is very low at our end, but if somebody creates a PR with one of the two solutions we will follow up on it.
If someone is even up for finding a solution for "private" constructors, which can still be used inside the module they are created in, please propose some code snippets here before investing time into a PR.

@kpalatzky
Copy link

@karfau In general i do have two aproaches in my mind to make the "constructor private":

Use of inheritance:

It would be possible to implement the "Interface" empty without a logic e.g. "Node" and additionally a "NodeImpl" which inherits from node and contains the logic. Only the "Node" but not the "NodeImpl" is exported. This allows you to check the node, but not create a new one.

Simple Example:
https://jsfiddle.net/kpalatzky/vb6m9urw/5/

Use of symbols:

A symbol is passed in the constructor, which is compared with an "internal symbol". If it is not the same, an error is thrown.
The internal symbol is not exported, so the constructor remains "private".

Simple Example:
https://jsfiddle.net/kpalatzky/vb6m9urw/8/

What do you think?

@Ponynjaa
Copy link
Contributor

@karfau I would be down to implement any of the two suggested approaches mentioned by @kpalatzky
Just tell me which you prefer. We really need these for our typings. Would be great for 0.9.0.

@karfau
Copy link
Member

karfau commented Mar 18, 2024

If the internal symbol solution is possible in ES5, it sounds like that is the solution which requires less code and is easier to follow.

karfau added a commit that referenced this issue Mar 26, 2024
Closes #40.

Exposes all fundamental and extended interfaces according to the
[specs](https://www.w3.org/TR/2000/REC-DOM-Level-2-Core-20001113/core.html)
while keeping the constructors for `Node`, `Document`, `Element`,
`Attr`, `CharacterData`, `Text`, `Comment`, `CDATASection`,
`DocumentType`, `Notation`, `Entity`, `EntityReference`,
`DocumentFragment`, `ProcessingInstruction` private by checking a symbol
in the constructor.

---------

Co-authored-by: Oliver Swienty <oliver.swienty@xpublisher.com>
Co-authored-by: Christian Bewernitz <coder@karfau.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation help-wanted External contributions welcome spec:DOM-Level-2 https://www.w3.org/TR/DOM-Level-2-Core/
Projects
Status: Done
7 participants