jwplayer / ott-web-app

Reference implementation for JWP-powered apps
Apache License 2.0
69 stars 53 forks source link

Feat / Extend minimum browser support #519

Closed langemike closed 1 month ago

langemike commented 2 months ago

We want to support a broader user base by extending our browser support to chrome68 and up.

Vite internally uses ['es2020', 'edge88', 'firefox78', 'chrome87', 'safari14']. So the only difference is the supported chrome browsers. See https://vitejs.dev/guide/build#browser-compatibility for reference.

I only needed to write a CSS fallback for the dvh unit because it was the only unsupported CSS feature we have ran into. This change also contains a variable injection fix for index.html which has been introduced (with the upgrade to vite 5, I assume).

We also did some internal experiments with @vitejs/plugin-legacy, but this change has a bigger impact on our bundle/build. If we even want to broaden our browser support further than chrome68 then @vitejs/plugin-legacy would definitely be a viable option.

I've tested this on Chrome 68 using browserstack.

Our original PR: https://github.com/Videodock/ott-web-app/pull/186 Our @vitejs/plugin-legacy experiment PR (for reference): https://github.com/Videodock/ott-web-app/pull/182

AntonLantukh commented 1 month ago

@langemike is "Chrome 68" something you need for Smart TV devices?

langemike commented 1 month ago

@langemike is "Chrome 68" something you need for Smart TV devices?

Correct!

ChristiaanScheermeijer commented 1 month ago

We discussed the knip ignore and virtual + relative path import via Slack and are planning to make the following (hopefully last) changes:

    resolveId(id) {
      if (id.includes('virtual:polyfills')) {
        return '\0' + id;
      }
    },
    load(id) {
      if (id.includes('\0virtual:polyfills')) {
        return enabled ? `import './polyfills';` : 'export default {};';
      }
    },
langemike commented 1 month ago

@ChristiaanScheermeijer I ammeded my previous commit and performed the approach you suggested.

I removed the ignore knip-config and put virtual:polyfills as a peerDependency to fix Knip's Unlisted dependencies warning.