Zirro / css-object-model

css-object-model is an implementation of the CSSOM specification
MIT License
9 stars 2 forks source link

Helping out #10

Open StefanoDeVuono opened 6 years ago

StefanoDeVuono commented 6 years ago

@Zirro

Hey I saw this project referenced in JSDOM's issue 2177 and I'd like to help out. Can you help me understand the API?

Zirro commented 6 years ago

Hey! Thank you, your help would be very appreciated. I had to put this project on hold due limited spare time and some challenges with underspecified behaviour in the CSSOM specification, but I'm happy to help new contributors get started. Here are the core parts:

Web IDL

Like jsdom and web browsers, this package uses a language known as Web IDL to describe the interfaces it implements from the specification. Here's an example of a .webidl file:

interface StyleSheet {
  readonly attribute CSSOMString type;
  readonly attribute USVString? href;
  readonly attribute (Element or ProcessingInstruction)? ownerNode;
  readonly attribute StyleSheet? parentStyleSheet;
  readonly attribute DOMString? title;
  [SameObject, PutForwards=mediaText] readonly attribute MediaList media;
  attribute boolean disabled;
};

This particular file describes the StyleSheet interface and its associated properties, which you will find in your browser as window.StyleSheet. When you run the yarn convert-idl script from package.json, this file is translated into a layer of JavaScript which automatically takes care of a lot of behaviour such as argument conversion. Once you've run it, you can find the files it created inside the lib/generated directory.

You will find all the .webidl files along with a corresponding *-impl.js file inside the interfaces directory.

Implementation

The impl-files is where we will write most of our code for each interface in order to comply with the specification. My advice for understanding them better would be to have a look at them and compare them with the .webidl files. In most cases, you'll find a getter and/or a setter for each property.

At this point it is time to look at the specification to find out what a property should do. It contains many step-by-step algorithms which often can be rewritten into code relatively easily. I would recommend following the specification as closely as possible.

Some reusable code is also located outside the interfaces folder, such as serializers and utilities. The latter is a bit scattered between a utils.js file and a utils/ directory at the moment, but I intend to sort this out eventually.

Much of the code is still in an incomplete state as you can tell, with TODO and FIXME comments located all over. I would recommend starting with smaller modifications to get into it.

Testing

Now this situation is a bit tricky. jsdom uses a large and constantly growing test suite known as the Web Platform Tests (WPT). Most of the relevant tests for this project can be found inside the css/cssom directory. While I worked on this project last year the amount of CSSOM-tests was a bit underwhelming, and some of them inaccurate. It looks like the situation might have improved with recent commits though.

The reason I say the situation is tricky is because this project has no way to run the tests on its own. It's dependent on jsdom to run the WPT suite, and thus requires the code which integrates css-object-model into jsdom. I have this code in my css-object-model branch which you'll need to checkout. Then you'll need to use yarn link to use your local version of css-object-model in the branch of jsdom. This situation will become a whole lot easier when jsdom eventually can start using css-object-model properly, but unfortunately we're not there yet.

Once you've done this you can run the tests in jsdom with the yarn test-wpt command. To limit the testing to the css/cssom directory, you'll want to use the --fgrep flag like so: yarn test-wpt --fgrep "css/cssom". Reading jsdom's Contributing.md could also help with this.

I hope you can get it up and running with these instructions, despite the current state of things. Let me know otherwise, and we'll try to figure it out.

?

I realise that there are probably several details missing above, so don't hesitate to ask if you have questions. I'd eventually like to make this project approachable for everyone.

Thanks again!

caub commented 6 years ago

for testing, you can just have jsdom and css-object-model (run npm run prepare or the equivalent yarn way) both cloned locally, and then from jsdom, you do yarn add ../css-object-model, I'll try to see how well this repo works

Zirro commented 6 years ago

@caub With yarn link (or npm link) you'll achieve the same result without having to modify package.json.

StefanoDeVuono commented 6 years ago

OK. So, I'm trying this out now. My steps are as follows:

Do you have insight into how to avoid these errors? Maybe there are some steps I'm doing incorrectly?

aphofstede commented 6 years ago

This worked for me:

See: https://yarnpkg.com/lang/en/docs/cli/link/ See: https://github.com/jsdom/jsdom/blob/master/Contributing.md#running-the-tests See: https://github.com/web-platform-tests/wpt#running-the-tests

caub commented 6 years ago

@StefanoDeVuono you should run npm run pretest && npm run test-wpt to update the generated files before testing

note you can do npm run pretest && npm run test-wpt --fgrep dom/nodes to test only on folder in wpt, else the whole test is pretty long

Also if you already have jsdom locally, you could just git remote add zirro git@github.com:Zirro/jsdom, and git checkout -b css-object-model zirro/css-object-model

edit: oh right, yarn doesn't play well with npm, I was locked at npm link ../css-object-model too

edit2: now it works with the PRs below, using npm

finally in jsdom css-object-model branch, you also need to remove the duplicate declaration here https://github.com/Zirro/jsdom/blob/css-object-model/lib/jsdom/living/navigator/Navigator.webidl#L43-L46

Zirro commented 6 years ago

Happy to see that you worked out a solution. I will try respond more quickly here if you have any questions in the future.

One of the larger projects at this point will be to update to the latest version of CSSTree. The API seems to have changed quite a bit since the version I last used here. While there seems to have been many improvements such as the array methods on List, there's a lot of breaking changes too.

Once I clear the current items on my TODO-list over the next few weeks, I'd be interested in returning to this project again. Let me know if any of you begin work on upgrading CSSTree, since I'll likely start there otherwise.