search

b-fuze / deno-dom

Browser DOM & HTML parser in Deno
https://jsr.io/@b-fuze/deno-dom
MIT License
423 stars 47 forks source link

Questions regarding contributions to this project #161

Closed lowlighter closed 6 months ago

lowlighter commented 7 months ago

Hi !

First thanks a lot for this awesome project

I'm currently using intensively deno_dom to test a back/front dom templating library. Since some behaviours are not mimicked/not implement, I'm currently monkey patching them in my codebase but I figured out that I could offer them upstream (such as #160 for .toggleAttribute) so everyone can benefit

However my question is which what's the exact scope of this project and what is accepted ? Some of these feature might be "niche" stuff I guess

I'm currently having some mocks for:

If this is somewhat within the scope of this project, I'll start submitting other pull requests in the next days

b-fuze commented 6 months ago

Hey, sorry for the late response.

Yeah, I'm generally okay with contributions. Relatively large contributions though I'd like if they could be discussed in some form. You could submit a PR but of course you know large PRs and complex changes will take a bit of time to review and discuss which might be okay depending on your availability. I will review your outstanding PRs now

lowlighter commented 6 months ago

Yeah I understand, I'm not planning on submitting large prs anyway since it's easier for both maintainers and contributors, whether it's for reviewing, reverting or amending

If I understood correctly the way deno-dom is designed, I'm assuming implementing specific behaviour would be by adding new elements in src/dom/elements (like for <template>, which seems the only one to be implemented currently).

So basically I was thinking on creating HTMLInputElement, HTMLTextAreaElement, HTMLSelectElement and HTMLOptionElement which would extend Element, and then I'd start implementing additional properties in them (the ones enumerated in first post, so value, selected, multiple and selectedOptions where they apply).

I'll check later for custom registry, since implementing it doesn't seem to difficult, but implementing the custom-elements are probably tricker and the solution I have currently is probably not compliant (I'm just calling the connectedCallback() right now since it's the only functionallity I need).

Assuming everything above is the way to go, there is still one point that'd be blocking (and tbh I don't really want to work on it since I don't know the implications and also would like to avoid touching too much on "core" code anyway), it's how elements are created

From what I've seen it occurs in two places: https://github.com/b-fuze/deno-dom/blob/944ba6b86cb7c2af407b9998216b4abe1c95fe55/src/dom/document.ts#L212 https://github.com/b-fuze/deno-dom/blob/944ba6b86cb7c2af407b9998216b4abe1c95fe55/src/deserialize.ts#L33

It seems this need to be (re)designed a bit, and also ask some questions on deno-dom's stance.

If more elements are implemented, then it'll definitely get bigger (and possibly slower because of more code, especially with more than 100 differents html elements) so people who like the fact that it's minimal and fast could be turned off. But since it wants to be compliant too, the fact it's lacking some "important" features (like value for <input> which is a core functionality for them) is troubling people that are searching for a reliant v-dom.

If it can give any insights on how some users are using deno-dom, I'll explain why I'm using it and for what purpose.

I'm writing isomorphic code (intended for browsers and runtimes environement) so I can perform both client/server side rendering, reliable test cases with code coverage that would probably be harder to achieve with other libs or an actual browser. The fact it's compliant with the spec is important for me since I can simulate more closely browser environement (which rule out linkedom) but the fact it's small, fast and typed gives it the advandtage against jsdom, which paradoxally may implement too much stuff... Since they're not implemented in deno-dom, I'm currently mocking custom elements (and registry), inputs and style attribute (with basic css parsing).

So of course I'd love these would be directly supported upstream, but I can understand also if it's out of scope too. The goals section seems that both POV are valable since it list "Fast" and "Mirror most supported DOM APIs as closely as possible".

Anyway I don't mind discussing directly on pr/issues as they're opened, since it makes it easier to sync whenever people have time, and it lets the discussion open if other persons have any inputs they'd like to share. But if needed I'm checking from time to time the deno discord

As a side note, Is there a tight dependency on deno (a part from the native version) ? Seems like the wasm version could be cross-runtime and I think deno-dom could have the potential to be a contender on other runtimes too

Sorry for the long text, hope it wasn't too boring 😅

b-fuze commented 6 months ago

@lowlighter on the specific points:

So basically I was thinking on creating HTMLInputElement, HTMLTextAreaElement, HTMLSelectElement and HTMLOptionElement which would extend Element, and then I'd start implementing additional properties in them (the ones enumerated in first post, so value, selected, multiple and selectedOptions where they apply).

Yeah, this is basically what I had in mind as well

I'll check later for custom registry, since implementing it doesn't seem to difficult, but implementing the custom-elements are probably tricker and the solution I have currently is probably not compliant (I'm just calling the connectedCallback() right now since it's the only functionallity I need).

Yeah, I'm going to be looking into this soon too hopefully as I also want to implement the new declarative shadow DOM API with <template>'s shadowrootmode

It seems this need to be (re)designed a bit, and also ask some questions on deno-dom's stance.

Yeah, this will basically be rewritten to be a big switch statement likely in a dedicated utility function somewhere so that hopefully the Javascript engine could apply the best optimizations possible.

As a side note, Is there a tight dependency on deno (a part from the native version) ? Seems like the wasm version could be cross-runtime and I think deno-dom could have the potential to be a contender on other runtimes too

The WASM version can work in other places and I've had people run it in service workers for instance. You are correct in that deno-dom is effectively not really coupled to Deno itself, and with the advent of JSR as I get some time I'll try to flesh out the library on that front as far as JSR being compatible with both Node and Bun for example. However, just to lower scope, I'm not trying to proactively make it "actually work everywhere" by e.g. publishing to NPM, providing unpkg links that work in a browser for example, etc...

I hope that helps

lowlighter commented 6 months ago

Yeah, I'm going to be looking into this soon too hopefully as I also want to implement the new declarative shadow DOM API

Really nice 👍

Yeah, this will basically be rewritten to be a big switch statement likely in a dedicated utility function somewhere so that hopefully the Javascript engine could apply the best optimizations possible.

Yeah I assume a switch shouldn't be too costly (I hope). At worst it'll probably just impact the startup time I guess.

The WASM version can work in other places and I've had people run it in service workers for instance. You are correct in that deno-dom is effectively not really coupled to Deno itself, and with the advent of JSR as I get some time I'll try to flesh out the library on that front as far as JSR being compatible with both Node and Bun for example. However, just to lower scope, I'm not trying to proactively make it "actually work everywhere" by e.g. publishing to NPM, providing unpkg links that work in a browser for example, etc...

Idk the actual compatibility of jsr with node/bun, but I assume just publishing this package to jsr is already a good push. I don't see why it'd become more intrinsicated with Deno anyways since there's not much I/O in deno-dom

Thanks for taking the time to answer

lowlighter commented 4 months ago

@b-fuze By any chance, did you already started looking into implementing CustomElementRegistry yet ?

Also is it ok if I open some PR to implement some behaviours on Input, TextArea, Select and Option ? I know that the mentioned refactor that hasn't occured yet, but I guess since it's only two places to patch it'll be fairly easy to refactor in the future ?

If not, it's ok, I can always rely on my fork until something get more fleshed out upstream, don't feel pressured to accept the pull requests if it doesn't suit the direction you'd like to take

b-fuze commented 4 months ago

No, I haven't had the time to work on custom elements yet. Yeah, I'm ok with PRs for those things, though <input> is a bit precarious because it is quite a complex element