WebReflection / linkedom

A triple-linked lists based DOM implementation.
https://webreflection.medium.com/linkedom-a-jsdom-alternative-53dd8f699311
ISC License
1.65k stars 79 forks source link

Export CSSOM classes #201

Open devingfx opened 1 year ago

devingfx commented 1 year ago

It can be usefull to be able to get and work with CSS classes, as it's already used in the library, why not make it accessible in root module and window like the rest of classes?

https://github.com/WebReflection/linkedom/blob/5527ee4aada82bd5cc07c739b87e372269922244/esm/html/style-element.js#L1

export * from 'cssom'
devingfx commented 1 year ago

PS: as a linkedom user in deno, I don't know how to use CSSStyleRule, etc... appart from importing a fresh new module that will conflict with the one used bt linkedom:

import { CSSStyleRule } from 'npm:cssom'
document.querySelector('style').sheet.cssRules[0].contructor === CSSStyleRule  
// false

I would need:

import { CSSStyleRule } from 'linkedom'
// or
import { cssom } from 'linkedom'
const { CSSStyleRule } = cssom
document.querySelector('style').sheet.cssRules[0].contructor === CSSStyleRule  
// true
WebReflection commented 1 year ago

this seems a reasonable question, I just need to be sure that module doesn't export conflicting names for the module. If you fancy a PR, please go ahead! (source of truth being the ./esm/ folder or any esm export in the package.json)

devingfx commented 1 year ago

After investing code, there is a conflict with CSSStyleDeclaration, you have one, and CSSOM have one other... without the same role nor apis ! :(

Your's is used to be synced with the ownerElement.style , and the one from CSSOM is the more generic one used to store also CSSRule.style and don't sync to anything.

Your's also is iterable and the other not...

Both are not fully compliant with W3C (like concerning constructed StyleSheet's .replaceSync ... )

So the thing is that you get 2 really different classes that should both be the same, depending on where you get it:

// el is a <style> with some css
assert( el.style.constructor == el.sheet.cssRules[0].style.contructor )
// false !
devingfx commented 1 year ago

I tried that to extract the classes from instances :

import { DOMParser } from 'https://esm.sh/linkedom@0.14.20'

const temp = ( new DOMParser ).parseFromString(`<style>
@import url("style.css") screen;
@media (min-width: 500px) {}
div {
    foo: bar;
}
</style>`, 'text/html' ).documentElement

const CSSStyleSheet = temp.sheet.constructor
const linkedomCSSStyleDeclaration = temp.style.constructor      // only Element.style , is part of linkedom
const CSSImportRule = temp.sheet.cssRules[0].constructor
const CSSMediaRule = temp.sheet.cssRules[1].constructor
const CSSStyleRule = temp.sheet.cssRules[2].constructor // or CSSRule
const CSSStyleDeclaration = temp.sheet.cssRules[2].style.constructor
const CSSRule = CSSStyleRule.prototype.constructor

export {
    CSSStyleSheet,
    CSSStyleDeclaration, linkedomCSSStyleDeclaration,
    CSSImportRule,
    CSSMediaRule,
    CSSStyleRule,
    CSSRule,
}
devingfx commented 1 year ago

My conclusion is that it's weird to have 2 different implementations of CSS parsing depending on using <style> or <elem style="">. linkedom should use fully the cssom module to parse style attribute!! As this is the only aspect of CSS of this library and also not its role...

This means rewrite the way CSSStyleDeclaration is synced to the ownerElement and rename this class to be a child of CSSOM's CSSStyleDeclaration and maybe re-implement the Proxy things ...

( level of skill that I don't have! ^^; )

WebReflection commented 1 year ago

It’s not a goal of this project to be 100% standard compliant, there’s JSDOM for that goal … I think I remember why indeed I didn’t use cssom internally, neither I’ve exposed that class externally.

as performance here matters more than standard compliance, what is your use case for having cssom available? If the answer is test then I’m afraid I’m not keen to accommodate that use case with changes that might both break and will slow down this project.

devingfx commented 1 year ago

I'm trying to re-implement a (compliant like) getComputedStyle for linkedom... but this means to implement a document.styleSheets that imply a way of loading/parsing <styles>s and <link rel=stylesheet>s and manipulate CSS classes.

1) I agree with you that's not the scope of linkedom (It's why I try to do it from outside of the library) and that's a good thing to opt-in getComputedStyle if needed only

2) I'm aware of JSDOM and other huge environments, but linkedom is fast! And so simple, no extra features, and that's good! Just need a basic CSS cascade resolver (that I did BTW, it's working bacause it use only W3C DOM specs and linkedom is fairly compliant on 99% ;) )

WebReflection commented 1 year ago

it's fast because I've pushed back on many similar requests (it misses just this, it misses just that, this should be more standard, ... and so on) but you gave me an idea around getComputedStyle as it passes through the windows proxy and it might be a decent guard to bootstrap more complex logic around CSS, hence use cssom when that's required.

I'll think about it, but not immediately.

devingfx commented 1 year ago

Well, I'm not sure if you took it hard (I'm not english native)... If it's the case please apologize!

I did not asked you to add this or that nor to be more W3C compiant ! :) I just asked "why not" exporting some already used bits... in fact, it is possible to just re-export cssom's exports, but as is, it could lead to unexpected weird bugs when using .style . (BTW it's possible to access the classes using instances as I wrote before)

The idea behind my request was to be able to code a custom getComputedStyle for my project... not to be integrated in linkedom not make you work on it ! ;)

And finaly... I may still work on this on my side (the same it's not my priority), if you want I may share future stuff / investigation here...

PS: The issue was more a way to communicate than a real issue though !

devingfx commented 1 year ago

it passes through the windows proxy and it might be a decent guard

I did'nt get why passing through the proxy is "a decent guard" ?

WebReflection commented 1 year ago

Apologies I’m not native English neither, let’s try to assume best intents from both side 😉

a guard meaning I can intercept the usage/need of getComputedStyle via the proxy and enable only when accessed a better cssom integration/story for style attributes and CSS parsing/behavior.

I’m short on time these days but I’ll have a look

devingfx commented 1 year ago

AFAIK I'm afraid t say that implementing a getComputedStyle may be a real chalenge because of the CSS cascade. There are a lot of concern (not even covered by cssom) to take in account, especialy selector precedence and DOM layout, pseudos element etc...
You may just make possible to plug each project's own implementation instead of integrate it... (In my case for ex. I don't need a full compliant browser selector precedence, just a "Object.assign()" kind of precedence in document inclusion order, to be honest, I could copy/paste a great part of JSDOM resolution algo )

The only real no go right now is the use of cssom "somewhere" but not "elsewhere" in linkedom, what's confusing when using a user-land own copy of cssom (or any other implementation).

1) The fastest and easiest solution for linkedom is to deprecate .style parsing and let the user make his own implementation... 2) The hardest way is to use cssom everywhere CSS parsing is needed, expose the classes, but as I said, this is more complicated that it sounds because of 2 version of CSSStyleDeclaration ...

Your choice BTW!

I'll be happy to help if you want me to start something (with your directives and good directions ;))

WebReflection commented 1 year ago

OK, I took the time to give cssom a shot and here's what's problematic:

I am skeptic about exporting cssom because of the CSSStyleDeclaration class conflict but, most importantly, because cssom doesn't know anything about this module and doesn't offer me any hook to react to changes or mutations, it would be an hazard to expose logic that won't be reflected (Mutation Observer or anything else) within this module.

Last, but not least, bundled files like the worker.js can't benefit from tree-shaking if I export all cssom when I need, in fact, only its parser, so that's another no-go from my side.

Accordingly with these findings and blockers, the only escape hatch I can think about is to offer an export linkedom/cssom that just points at cssom and that would be it: you can use the same module exported in here + you are on your own with extra logic ... would that work?

devingfx commented 1 year ago

I can to almost the same conclusions:

So I came to the question:

Why parsing <style>.sheet tough? Is it so needed for SSR? (I'm not used at every frameworks like react, et cie) Can't it be removed, removing the same way 3rd party dependency. Just disclaim that CSS parsing is a user-land feature... ( I mean keeping .style parsing untouched because it is not realted to cssom at all)

WebReflection commented 1 year ago

Why parsing