GoogleChromeLabs / css-triggers

A reference for the render impact of mutating CSS properties.
https://csstriggers.com/
Apache License 2.0
902 stars 70 forks source link

Fix static cache #26

Closed surma closed 8 years ago

surma commented 8 years ago

R @oldergod: Would appreciate a 2nd pair of eyes :)

@paullewis as FYI

oldergod commented 8 years ago

Code looks good to me. Tested it locally and works fine.

If you don't mind me asking, what's going on with the dynamic cache? I understand that with addAll, if one request fails, nothing get cached so you do it one by one but why are you doing it manually?

Is there much difference between

// https://github.com/GoogleChrome/css-triggers/blob/ec038ae6a54acb0b1b0b104e892cce3b35a3b916/src/scripts/sw.js#L54
return DYNAMIC_FILES.map(url => {
  return fetch(url)
    .then(
      response => cache.put(url, response),
      err => {}
    );
});

and

return DYNAMIC_FILES.map(url => cache.add(url));

?

surma commented 8 years ago

Ugh, there was a reason, but I can’t quite recall. I’ll talk to @paullewis if he remembers and add a comment.

paullewis commented 8 years ago

Yeah the 404 is an error causing the install step to fail. We ignore the failure IIRC. We should a comment

surma commented 8 years ago

Ah yes! We want to cache /404.html, which Firebase backend will deliver with a 404 error code but we still want to cache that. So that’s why we have written our own thing here. I will add a comment.

surma commented 8 years ago

See 03f3a51

oldergod commented 8 years ago

Thank you for the explanation. That makes sense!