♻️ Use JSS for video

[alanorozco]

This is a port of video-autoplay.css.

No CSS def in an AMP component since there's not a PreactBaseElement that uses VideoWrapper yet.

[alanorozco]

@samouri Do I need to unwrap this loop since the member access for classes is dynamic?

[alanorozco]

(Current compiled output seems ok btw. In the future we could optimize this away as to not read classes, which would then break this.)

[samouri]

Context: I have a PR coming up that converts the classes object to an enum.

You don't need to unroll the loop, but if you don't it'll deopt. I just tested it via the appspot closure compiler.

static access (unrolled)

// Source file
/** @enum {string} */
const a = { cx1: 'hello', cx2: 'world'};

console.log(a.cx1);
console.log(a.cx2)

// Compiled output
console.log("hello");
console.log("world");

dynamic access

// Source file
/** @enum {string} */
const a = { cx1: 'hello', cx2: 'world'};

[1,2].forEach(i => {
  console.log(a[`cx${i}`]);
})

// Compiled output
var a = {a:"hello", b:"world"};
[1, 2].forEach(function(b) {
  console.log(a["cx" + b]);
});

[dvoytenko]

Yeah, we wanted to make it fully enumerable so that it can be fully DCE'd to constants.

[alanorozco]

Instead of unrolling, I wrote a compound selector. I believe it can be DCE'd now.

[alanorozco]

Or... the current solution to map only those classes one level above :)

[alanorozco]

Or... just nth-child(x):before, :after :)

[samouri]

i like the sound of this color 🎵

[samouri]

FMI, do we have any data on the performance impact of using will-change?

[alanorozco]

No actual data. It does have an impact, since it definitely prevents some browsers from re-rendering when changing other CSS props.

[samouri]

whats the point of keeping amp-video-eq-col? Should probably just use eqCol instead right (can also update the eqPlaying rule above)?

[alanorozco]

Technically this element's style is overridable via this class. Though I'm not sure any author has gone that deep 🤷‍♂️

[samouri]

Ok cool! Just making sure theres a reason. Do you know if this will live in the shadow or light dom? If lightdom then it is worth keeping as-is, if shadow then I'm not sure they can customize it via classname anyway.

[alanorozco]

It's using ['children'], so it's rendered in ShadowDOM. Though its children are cloned into VNodes, so we could move it to the light DOM. In any case, I think it's fine to remove access to amp-video-eq-col, but maybe not to amp-video-eq since that may be used to hide the equalizer...

[dvoytenko]

IMHO this looks very clean.