Add a Lit TodoMVC app

[rictic]

This aims to fix https://github.com/WebKit/Speedometer/issues/52

[flashdesignory]

@rictic - lint check is failing. A good way to catch things is by running npm run format from Speedometer's root locally before pushing.
Speedometer has it's own prettier / eslint rules for consistency between workloads and to get as close as possible to Webkits style guide.

[rictic]

Huh, I think something is wrong with my prettier setup, as npm run format is making many changes to code not changed in this PR. Investigating.

[rictic]

I was definitely doing something wrong. This should now be properly formatted and passing eslint

[flashdesignory]

@rictic - I noticed that you are defining the styles inside js, which is more idiomatic to lit, but doesn't let us manage consistency in the styles across todomvc apps. This workload also looks slightly different compared to the other ones.

I added an alternate approach with css-modules here (if approved): https://github.com/WebKit/Speedometer/pull/254

these could be installed as a local package and that could potentially work? Note: these styles expect a slightly different dom structure.

Just posting here for thoughts.

[flashdesignory]

This build looks great and it's pretty easy to follow along and parsing the code 🚀 .

The only thing that threw me off was that the Todos class also handles route changes. I'd expect the app to listen for those and notify or set the filter in the Todos class., but that's maybe subjective.

Besides that, the only other difference I noticed was here the todos id is a simple int that increments, vs other implementations where we use uuid / nanoid.

[rictic]

There's a bunch of distinct things that people commonly call CSS modules, would we be able to import them and get a constructable stylesheet? If so, it'd be easy to integrate into a Lit app or just about any web component, and I'd be happy to update this PR with it.

I was split on putting the routing in the Todos class, an earlier draft actually did put it into <todo-app>'s code. Either seems fairly reasonable to me tbh, happy to move it if you'd like.

Good catch on the ids, I'll check out nanoid.

[rniwa]

CSS modules

It's probably best to refrain from using features that are not universally available. Otherwise, we'd be comparing apples to oranges across browsers.

[flashdesignory]

CSS modules

It's probably best to refrain from using features that are not universally available. Otherwise, we'd be comparing apples to oranges across browsers.

I should have clarified - I assumed there's support for css loaders that would allow us to import css modules.

[flashdesignory]

I don't know lit well enough, but for example something like: https://www.npmjs.com/package/lit-css-loader

[rictic]

@rniwa yeah, not proposing anything that isn't shipping cross browser, but it's easy to do build time work to get a JS module that exports a constructible stylesheet.

https://github.com/Alorel/rollup-plugin-constructable-css looks like a nice API

[flashdesignory]

@rniwa yeah, not proposing anything that isn't shipping cross browser, but it's easy to do build time work to get a JS module that exports a constructible stylesheet.

https://github.com/Alorel/rollup-plugin-constructable-css looks like a nice API

thanks @rictic - i looked at the plugin and it's pretty straight forward. I'll add it

[flashdesignory]

@rictic - turns out that I couldn't install it.. I did write a super simplified plugin to support your use-case though (at least I hope that's what you need). https://github.com/WebKit/Speedometer/pull/254

the dist output folder now has constructable files (app.constructable.js, ect...).

lmk - happy to keep modifying the plugin until it gives you what you need.

[rictic]

The .constructable.js files look like they'll work great. We can replace the styles in the static styles arrays with those.

[rictic]

Let me know if there's anything I can do here to help this to land, I'm very much interested in getting this into Speedometer 3!

[flashdesignory]

Let me know if there's anything I can do here to help this to land, I'm very much interested in getting this into Speedometer 3!

Sounds good - will review today

[flashdesignory]

@bgrins @rniwa - Do we want to land this pr and deal with external css / constructable css mod later? Don't want to block this pr and it feels like it's almost there.

[julienw]

Because this wasn't super clear for everybody, the lit code already uses socalled "constructable css" with its css template tag, and uses adoptedStyleSheets under the hood. The related code is: https://github.com/lit/lit/blob/3711e6650a59966e5be8d92dd0abf053d9a50d32/packages/reactive-element/src/css-tag.ts

[rictic]

Yeah, expanding on that, the css template tag returns a CSSResult. We'd like it to just return a CSSStyleSheet, but we support older browser versions that didn't yet have constructible stylesheets. I could update this example to only use CSSStyleSheets, a LitElement's styles can take a CSSStyleSheet. I went with this approach because it's the most common way that Lit code is written in the wild.

[julienw]

Yeah, expanding on that, the css template tag returns a CSSResult. We'd like it to just return a CSSStyleSheet, but we support older browser versions that didn't yet have constructible stylesheets. I could update this example to only use CSSStyleSheets, a LitElement's styles can take a CSSStyleSheet. I went with this approach because it's the most common way that Lit code is written in the wild.

Yes, this sounds good to me! I just wanted other folks to know that this uses adoptedStyleSheets under the hood (for our current browser versions).