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

Proof of concept querySelector helpers #13

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Proof of concept querySelector helpers #13

wants to merge 3 commits into from

Conversation

nojaf
Copy link
Collaborator

@nojaf nojaf commented Nov 18, 2024

Somewhat related to #5

I'd like to experiment a bit with the same trick that was used in the original webapi bindings:

module Impl = (
  T: {
    type t
  },
) => {

Some duplication in the methods, which currently exists, can be avoided by this.

@zth
Copy link

zth commented Nov 18, 2024

My 5c is that we should avoid reusing that trick. In my view it just makes the code more complicated and hard to follow for little gain.

@nojaf
Copy link
Collaborator Author

nojaf commented Nov 18, 2024

There are currently about 60 files that use querySelector. Adding a safe version for each element type could result in significant duplication.

@zth
Copy link

zth commented Nov 18, 2024

Well, using include creates that same duplication in the compiler anyway (include is a copy paste in the compiler). It's not in the ReScript source files, but it'll be the same duplication in the output. And the output will probably be worse and harder to follow.

What about just using functions? Some internal function that takes a classifier or similar for each use case. Then we can just partially apply that same function in all relevant places. If we worry about duplication.

@nojaf
Copy link
Collaborator Author

nojaf commented Nov 18, 2024

I don't think I mind duplication, but in this case, there is practical side.
I would add about 60 helpers for type safe querySelector access and would need to add those in 60 files.
The include trick makes this that second part easier. Especially if we revisit bindings.

What about just using functions? Some internal function that takes a classifier or similar for each use case. Then we can just partially apply that same function in all relevant places. If we worry about duplication.

Could you give an example maybe?

@zth
Copy link

zth commented Nov 18, 2024

Just so I understand correctly - you mean each of these 60 files get 60 variations of querySelector? So we have 3600 bindings to queryselector essentially?

@nojaf
Copy link
Collaborator Author

nojaf commented Nov 18, 2024

Yeah, so you have document.querySelector which returns an element.
In this first example I've added a querySelector_htmlCanvasElement helper to the document module.
But any HTMLElement interface has the querySelector method as well, because of the Element inheritance.

The way it is currently set up is that HTMLButtonElement for example, currently has a @send querySelector taking an htmlButtonElement as first argument. This is to avoid that the user would need to "downcast" the htmlButtonElement to an element before they can use querySelector.

@nojaf
Copy link
Collaborator Author

nojaf commented Nov 18, 2024

@zth I've added an example to illustrate this in 374f47c

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

Successfully merging this pull request may close these issues.

2 participants