Ideal jsdom CSS implementation architecture

[domenic]

Continuing discussion from #234 (comment).

I think the ideal architecture is maybe something like this?

• At the foundation is a well-maintained third-party CSS library, maybe csstree or things under @csstools/, for core things like parsing.

• All Web IDL-generated classes except CSSStyleDeclaration live in jsdom/jsdom, because that is where our webidl2js infrastructure is. They are all relatively simple and delegate hard CSS-specific stuff like parsing, serialization, etc. to another library.

  • This includes: StyleSheet, MediaList, CSSStyleSheet, CSSRule, lots of CSSRule-derived classes
  • For example, maybe the parsing library produces a "css import rule" AST structure, with properties like href, layerName, and supportsText. The jsdom code for CSSImportRule would wrap around that AST structure to implement the href getter (including URL resolution), layerName and supportsText getters (just passthrough), and media and styleSheet getters (via its reference to the containing CSSStyleSheet).

• In the middle there are jsdom-specific libraries that takes care of any hard CSS logic (not CSSOM object implementation) that the core third-party library does not help with. For example, @asamuzakjp/css-color or cssstyle

• I am not sure where the implementation of CSSStyleDeclaration and CSSStyleProperties should live. Maybe they can be in jsdom/jsdom and be wrappers around the jsdom-specific library. Or maybe they should be in the jsdom-specific library instead.

Part of the motivation to put more of the classes in jsdom/jsdom is that then the other libraries don't have to worry about the Web IDL wrapper/impl distinction and the accompanying build process. It's possible to have parts of jsdom outside of jsdom/jsdom use webidl2js. (For example, whatwg-url does this.) But the pattern of doing the wrapping in jsdom and the core logic in another library is, in my opinion, simpler. (See, e.g., w3c-xmlserializer + jsdom/jsdom wrapper.)

The above seems pretty different from what I understand of our current architecture. Our current architecture has the parser produce CSSOM objects like CSSImportRule directly.

It's possible the above suggestion is bad, or is failing to grasp the hard part of our CSS stack. Better ideas are welcome.

[asamuzaK]

I've changed cssstyle to parse using css-tree.
I haven't looked deeply into the code of rrweb-cssom or the @acemir/cssom, but I think that it would be a good idea for them to use css-tree as well.
Because css-tree allows you to flexibly specify context, e.g. StyleSheet, CSSStyleSheet, CSSRule.
https://github.com/csstree/csstree/blob/master/docs/parsing.md#context

• I am not sure where the implementation of CSSStyleDeclaration and CSSStyleProperties should live. Maybe they can be in jsdom/jsdom and be wrappers around the jsdom-specific library.

I want to make sure I understand your suggestion correctly, if we decide to live CSSStyleDeclaration in jsdom/jsdom, would it be something like this?

lib/jsdom/living/cssom/CSSStyeDeclaration-impl.js:

"use strict";

const cssstyle = require("cssstyle");

exports.implementation = class CSSStyleDeclarationImpl extends cssstyle.CSSStyleDeclaration {
  constructor(globalObject, args, privateData = {}) {
    const { context, onChangeCallback } = privateData;
    super(onChangeCallback, { context, globalObject });
  }

  get length() {
    return super.length;
  }

  // ... and so on
};

Also, there is #116

[domenic]

I want to make sure I understand your suggestion correctly, if we decide to live CSSStyleDeclaration in jsdom/jsdom, would it be something like this?

Basically yes. But, I was thinking of using encapsulation instead of inheritance. This would allow the inner object to be simpler and not concerned with tricky stuff like indexed properties or named setters. Something like this:

// CSSStyleDeclaration-impl.js
"use strict";

const cssstyle = require("cssstyle");
const { mixin } = require("../../utils.js");
const CSSStylePropertiesImpl = require("./CSSStyleProperties-impl.js").implementation;

exports.implementation = class CSSStyleDeclarationImpl {
  constructor(globalObject, args, privateData = {}) {
    const { context, onChangeCallback } = privateData;
    this._styleDeclaration = new cssstyle.StyleDeclaration(onChangeCallback, { context, globalObject });
    this._initCSSStyleProperties();
  }

  get length() {
    return this._styleDeclaration.declarations.length;
  }

  item(i) {
    return this._styleDeclaration.declarations[i]?.name ?? "";
  }

  getPropertyValue(property) {
    // Maybe implement the whole algorithm in https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-getpropertyvalue
    // over this._styleDeclaration.declarations? Or maybe just
    return this._styleDeclaration.getPropertyValue(property);
  }

  // ... and so on
};

mixin(exports.implementation.prototype, CSSStylePropertiesImpl.prototype);

// CSSStyleProperties-impl.js
"use strict";
const cssstyle = require("cssstyle");

exports.implementation = class CSSStylePropertiesImpl {
  get cssFloat() {
    return this.getPropertyValue("float");
  }
  set cssFloat(value) {
    return this.setPropertyValue("float", value);
  }
};

for (const [idlAttribute, cssProperty] of cssstyle.supportedProperties) {
  Object.defineProperty(CSSStylePropertiesImpl.prototype, idlAttribute, {
    get() {
      return this.getPropertyValue(cssProperty);
    }
    set(value) {
      this.setPropertyValue(cssProperty, value);
    }
  });
}

Also, there is #116

Yes. #116 is a generally good idea. I did not have bandwidth to review it at the time, and still do not really. If you are able to revive it to a level you think is reasonable, I can provide minimal review and trust that you are happy with it.

However, note that it takes the approach which I think is probably not ideal, i.e.,

have parts of jsdom outside of jsdom/jsdom use webidl2js. (For example, whatwg-url does this.)

Libraries that take this approach, like whatwg-url or #116, end up needing a somewhat heavyweight build process, and have to concern themselves both with core functionality and with getting perfect IDL bindings. I think letting jsdom/jsdom worry about the IDL parts, and just having this library implement really good core semantics and export an IDL attribute <-> CSS property mapping, might be simpler.

But, this is my speculation on what is best. Whoever actually does the work, should feel free to decide on the direction.

Back when @ExE-Boss was more active in the jsdom project, I was more active and insistent on architecture being perfect and on getting full review on everything. I still feel that way to some extent the about core jsdom/jsdom. But for jsdom/cssstyle, I recognize these days I don't have time for serious investment, and so I want to let others like yourself set the direction. (Or @ExE-Boss, if he is still interested.)

[asamuzaK]

I was thinking of using encapsulation instead of inheritance. This would allow the inner object to be simpler and not concerned with tricky stuff like indexed properties or named setters.

I see.
Thanks for clarification.

[asamuzaK]

Re: #234 (review)
My goal is to fix window.getComputedStyle().
Here are my plans for future cssstyle updates toward that goal:

• properties/*.js
  • Replace isValid() with isValidPropertyValue() #236
  • Rewrite parsers to be AST based #237
• parsers.js
  Currently cssstyle always returns specifiedValue.
  Make it possible to return computedValue when CSSStyleDeclaration is called from window.getComputedStyle().
  Also, check the external values required for this change (fontSize, window.width, window.height, baseURL, currentColor, etc.)
  • Add ability to return computed value #240
• generated/implementedProperties.js
  Once Use @webref/css #229 is resolved, export implemented properties with it's definitions (initial, computed, inherited, etc.), and replace propertiesWithResolvedValueImplemented in living/helpers/style-rules.js of jsdom.
  • Export implemented properties with definitions #241