Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use modern build for ipad os 15 and macos 11 #21695

Closed
wants to merge 1 commit into from
Closed

Use modern build for ipad os 15 and macos 11 #21695

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions build-scripts/gulp/entry-html.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,29 @@ import { htmlMinifierOptions, terserOptions } from "../bundle.cjs";
import env from "../env.cjs";
import paths from "../paths.cjs";

// Regex to match iPadOS >= 15 and macOS >= 11 (app web view) because browserlist is not capable of this
const ipadMacOSRegex =
/\b(iPadOS\s*(1[5-9]|[2-9]\d)(?:\.\d+)?(?:\.\d+)?)|(macOS\s*(1[1-9]|[2-9]\d)(?:\.\d+)?(?:\.\d+)?)\b/;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot hard code the versions because they can change with time and/or usage stats. I'm looking into the best solution here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be we should also put Safari 16 in browserlist file to have the same target in browserlist. I agree it's not perfect but I didn't find another solution 😢

Let's me know if you find something better!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've put in many hours researching this now, and the only solid conclusion I've got is that Apple needs to do better. 😞

For iPad, I think home-assistant/iOS#2918 is the best solution (and looks like @bgoncal just merged it as I'm typing so 🥳). There's no reason we need to allow the app to spoof the user agent to pretend it's a Mac because it won't affect the frontend result at all. So the current regex will work now for iPad at least.

For the Mac app, that's a complete mess. I had a discussion with @bgoncal and continued to research, and the only version provided by Apple's API is the OS version (i.e. there's no way to query what WebKit or Safari version gets used by the web view). And it's not provided in the default user agent string either. So the only choice is to map it ourselves. The only question for me becomes: should the MacOS version map to the Safari version it's shipped with, or the latest version it supports? Based on my research, I think the former is correct, but I'm not 100% sure on that.

I started looking into how to automate this similar to the way browserslist-useragent-regexp works, and I think I could probably throw that together without too much trouble. It's late here now so I'll do that tomorrow.

Regarding putting more Safari versions in the browserslist config, I agree. Currently, modern is served to iOS 15.6+ and Safari 17+, which doesn't make much sense from a transpilation/polyfilling perspective. That's just reflecting that iPhones are way more popular than Macs. We can hard code a sync for now and maybe I'll do another PR to browserslist for a nicer solution.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree! Thank you for the research! 🙂
That was more or less my conclusion too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened browserslist/browserslist#838. I'll work on the change for Mac in a bit.


const renderTemplate = (templateFile, data = {}) => {
const compiled = template(
fs.readFileSync(templateFile, { encoding: "utf-8" })
);

const userAgentRegex = getUserAgentRegex({
env: "modern",
allowHigherVersions: true,
mobileToDesktop: true,
throwOnMissing: true,
});

return compiled({
...data,
useRollup: env.useRollup(),
useWDS: env.useWDS(),
modernRegex: getUserAgentRegex({
env: "modern",
allowHigherVersions: true,
mobileToDesktop: true,
throwOnMissing: true,
}).toString(),
modernRegex: new RegExp(
`(${ipadMacOSRegex.source})|(${userAgentRegex.source})`
).toString(),
// Resolve any child/nested templates relative to the parent and pass the same data
renderTemplate: (childTemplate) =>
renderTemplate(
Expand Down
Loading