Use rewrite for placeholders and improve cache control

[steverep]

Proposed change

This makes changes to the Netlify config to fix the referenced issues:

• Condenses the redirects table into just 2 entries for brevity, and removes the headers keys which are not applicable (explained more in Condense redirects using placeholders and remove request headers #4685)
• Uses rewrites instead of redirects for placeholder fallback, i.e. return 200 instead of 302
  • Skips the extra network requests resulting in a performance bump
  • Unblocks issues with CORS requests in frontend service work as discussed in Placeholder redirects and CORS don't mix #4707
  • Minor disadvantage is that placeholders cannot have separate headers applied for cache control; however, this seems like an acceptable compromise given that it really only helps for domain additions and not other changes. This is addressed further in the next point.
• Apply a single set of cache control directives for all assets, with adjusted ages and revalidation policy as discussed in Browser cache life is too short and restricted #4718
  • Cache in browsers for 7 days for good browser performance and internet outage protection
  • Cache on Cloudflare for 24 hours to pick up any change relatively quickly, not just full domain additions (users reloading then guaranteed < 1 day old data)
  • Apply stale-while-revalidate strategy for another 7 days for browsers that support it (Firefox and Chrome at the moment); others will use their internal heuristics

Type of change

• Add a new logo or icon for a new core integration
• Add a missing icon or logo for an existing core integration
• Add a new logo or icon for a custom integration (custom component)
  • I've opened up a PR for my custom integration on the Home Assistant
    Python wheels repository
• Replace an existing icon or logo with a higher quality version
• Removing an icon or logo

Additional information

• This PR fixes or closes issue: fixes Placeholder redirects and CORS don't mix #4707 and fixes Browser cache life is too short and restricted #4718
• Link to code base pull request:
• Link to documentation pull request:
• Link to integration documentation on our website:

Checklist

• The added/replaced image(s) are PNG
• Icon image size is 256x256px (icon.png)
• hDPI icon image size is 512x512px for ([email protected])
• Logo image size has min 128px, but max 256px, on the shortest side (logo.png)
• hDPI logo image size has min 256px, but max 512px, on the shortest side ([email protected])

[frenck]

This change seems incomplete.

• The PR description mentions rewrites instead of redirects, but these are not in this PR?
• Removed redirect have no replacement. For example, all the fallback logic is done using redirects, but those have been completely removed and not replaced. For example dark_logo falls back to logo using a redirect; but has been removed without replacement.
• Redirects are throwing a status 200 now, which is not correct in the HTTP specification.

../Frenck

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[steverep]

Rewrites are created in Netlify by returning 200 from a redirect rule. It's the equivalent of a rewrite in a .htaccess on Apache for example.

The fallback behavior is exactly the same. The only difference is the URL doesn't change in the address bar, which is inconsequential.

[frenck]

The fallback behavior is exactly the same.

It isn't.

For example, from the old rules:

[[redirects]]
  from = "/brands/_/*/dark_logo.png"
  to = "/brands/_/_placeholder/logo.png

Nothing is replacing any of that logic.

../Frenck

[steverep]

That's replaced by:

[[redirects]]
  from = "/brands/_/*/:image"
  to = "/brands/_/_placeholder/:image"
  force = false
  status = 200

It rewrites to .../_placeholder/dark_logo.png, but that's the same image (and allows putting in dark placeholder variants w/o changing config if desired).

These links return the same placeholder image:

• https://brands.home-assistant.io/brands/_/steverep/dark_logo.png
• https://deploy-preview-4722--home-assistant-brands.netlify.app//brands/_/steverep/dark_logo.png

If there's something not working here, I'm happy to fix it, but I don't see any problems when testing the deploy preview.

[frenck]

That's replaced by:

That is not a replacement. Again:

[[redirects]]
  from = "/brands/_/*/dark_logo.png"
  to = "/brands/_/_placeholder/logo.png

Is breaking/missing in this PR.

Right now, if the dark logo doesn't exists, it will fallback to the non-dark version. Your version doesn't do that.

../Frenck

[steverep]

Right now, if the dark logo doesn't exists, it will fallback to the non-dark version. Your version doesn't do that.

😕 The rule you are quoting doesn't do that either - it falls to the placeholder's non-dark version. But that only happens if the domain doesn't exist. For domains that exist, the build script is handling that logic by making copies when variants are missing. All 8 variants exist if the domain exists with at least an icon.

Maybe I'm missing something though. Can you give me a deploy preview link that is not working correctly?

[steverep]

To try to be ultra-clear on the rule you claim is broken:

Branch	From	Action	To	Result
Current	/brands/_/*/dark_logo.png	Redirect	/brands/_/_placeholder/logo.png	Copy of core_integrations/_placeholder/logo.png
PR	/brands/_/*/dark_logo.png	Rewrite	/brands/_/_placeholder/dark_logo.png	Copy of core_integrations/_placeholder/logo.png (since there's no dark placeholders)

In other words, the current rule for dark variants is redundant with the build script: # Create fallback for dark variantsis . This PR is just removing that redundancy. Here's the deploy preview link working just fine for Epson (which does not have dark variants in the repo):

https://deploy-preview-4722--home-assistant-brands.netlify.app//brands/_/epson/dark_logo.png

[steverep]

@frenck I know you're probably busy with the stream today, but could we please resolve this when you get a chance? I think if you actually test the Netlify preview, you'll see the placeholder fallbacks are working correctly.

[steverep]

Nearly a month and 90 subsequent PRs have followed this one, the majority of which were promptly merged or approved without much wait. I get this PR isn't simply adding some images, but it is far from an overwhelming and taxing change, and it will benefit all users. Getting this across the finish line would be appreciated.

[frenck]

Nearly a month and 90 subsequent PRs have followed this one, the majority of which were promptly merged or approved without much wait.

You are right, this is not a PR I review I would review from my tablet, the other I would (and do). So, yeah.

[frenck]

To try to be ultra-clear on the rule you claim is broken:

That is really helpful! The part that didn't exist in my mind was:

brands/scripts/build.sh

Lines 113 to 121 in ce098e0

	# Create fallback for dark variants
	find ./build -type f -type f -name "icon.png" -o -name "[email protected]" -o -name "logo.png" -o -name "[email protected]" | while read image; do
	dir=$(dirname "${image}")
	filename=$(basename -s .png "${image}")
	if [[ ! -f "${dir}/dark_${filename}.png" ]]; then
	cp "${image}" "${dir}/dark_${filename}.png"
	echo "Using ${image} as dark_${filename}"
	fi
	done

So, thanks for pointing that one out.

[frenck]

Cannot find a single thing that doesn't work as expected on the Netlify preview. So 👍

../Frenck