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

Circular dependencies, recursive types and type spreading #2

Open
nojaf opened this issue Oct 26, 2024 · 9 comments
Open

Circular dependencies, recursive types and type spreading #2

nojaf opened this issue Oct 26, 2024 · 9 comments

Comments

@nojaf
Copy link
Collaborator

nojaf commented Oct 26, 2024

As most of us, may have somewhat expected, the DOM has cyclic dependencies between types.

Example case:

Types Window, Document and Node.

The property window.document means type document needs to be known before type window.

Document inherits from Node, so Node needs to be known to Document to be able to spread.

Node.ownerDocument returns a document from node thus creating a loop.

type rec node = {
    ownerDocument: Null.t<document>,
}

and document = {
    ...node
}

and window = {
    document: document;
}

/*
Syntax Errors
[E] Line 10, column 22:
Did you forget a `,` here?
 */

Spreading does not work here and duplicating the properties leads to

type rec node = {ownerDocument: Null.t<document>}

and document = {ownerDocument: Null.t<document>}

and window = {document: document}

// Compiled with 1 Warning(s)
// [W] Line 3, column 16:
// the label ownerDocument is defined in both types node and document.

It is my understanding that https://github.com/TheSpyder/rescript-webapi uses all the types defined in node_modules/rescript/lib/ocaml/dom.ml, I can no longer find this file in the current ReScript compiler though. It is my understanding that we also no longer what to use that.

Anyway, feels like back to the drawing board.
Suggestions to model this are welcome!

@zth
Copy link

zth commented Oct 26, 2024

We can turn off that warning locally in the file(s) as needed:

@@warning("-30")

type rec node = {ownerDocument: Null.t<document>}

and document = {ownerDocument: Null.t<document>}

and window = {document: document}

This yields no warning. So that would be an acceptable approach still I think.

But, it'd be good to only duplicate props only when needed, and still spread them when we're not in a recursive chain.

@zth
Copy link

zth commented Oct 26, 2024

It is my understanding that https://github.com/TheSpyder/rescript-webapi uses all the types defined in node_modules/rescript/lib/ocaml/dom.ml, I can no longer find this file in the current ReScript compiler though. It is my understanding that we also no longer what to use that.

The file is here now: https://github.com/rescript-lang/rescript-compiler/blob/master/runtime/Dom.res

@nojaf
Copy link
Collaborator Author

nojaf commented Oct 28, 2024

Thanks for pointing that out. I believe we will need an initial chain of recursive types where we repeat the properties. Once we have those, we can have some edge types which can use spreading.

@zth
Copy link

zth commented Oct 28, 2024

Thanks for pointing that out. I believe we will need an initial chain of recursive types where we repeat the properties. Once we have those, we can have some edge types which can use spreading.

Sounds like a good approach!

@nojaf
Copy link
Collaborator Author

nojaf commented Oct 30, 2024

Alright, when it comes to the interfaces I believe we have three categories: the precursors, the chain and the spreaders.

The precursors are interfaces which are used in the chain and need to be defined first.
They are pretty standalone I believe.

Examples:

"DOMTokenList",
"NamedNodeMap",
"Location",
"FragmentDirective",
"DocumentTimeline",
"History",
"CustomElementRegistry",
// https://developer.mozilla.org/en-US/docs/Web/API/Window/locationbar
"BarProp",
"Navigator",
"Screen",
"VisualViewport",
"SpeechSynthesis"

Then there is a chain of recursive types (which thus cannot use spreading)

Examples:

"Node",
"NodeList",
"Element",
"ShadowRoot",
"HTMLElement",
"HTMLCollection",
"HTMLHeadElement",
"DOMImplementation",
"DocumentType",
"Document",
"Window",

The chain has circular dependencies and needs to copy all properties of its base interfaces.
Once that chain is established, we will be able to use spreading again.

Example:

HTMLButtonElement
HTMLInputElement
...

@zth
Copy link

zth commented Oct 30, 2024

One question that comes to mind - do we have circular dependencies in the spreads too? Or is the problem mainly that spreads, as of now, simply aren't allowed in recursive definitions?

If it's the latter, one could look into what it'd take to allow spreading in recursive definitions in the compiler, provided we can find a way to error out if the spread happens to be circular.

@nojaf
Copy link
Collaborator Author

nojaf commented Oct 30, 2024

Good point, I would need to check that.

@cristianoc
Copy link

I think it would be pretty interesting to just describe what ideal typing for DOM would look like, to begin with, and separately check what is supported today and what might be needed in terms of extensions.
I'm not even sure the DOM has ever been typed properly at all.

@nojaf
Copy link
Collaborator Author

nojaf commented Oct 30, 2024

do we have circular dependencies in the spreads too?

It appears that https://developer.mozilla.org/en-US/docs/Web/API/CSSStyleSheet and https://developer.mozilla.org/en-US/docs/Web/API/CSSRule have a nested thing going on.

This is not necessarily a problem, it might be fine to have multiple chains that need to be emitted in order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants