Skip to content
This repository has been archived by the owner on Aug 4, 2024. It is now read-only.

RFC: support f(browser) => array-of-missing-features #1115

Closed
cdaringe opened this issue Nov 10, 2021 · 6 comments
Closed

RFC: support f(browser) => array-of-missing-features #1115

cdaringe opened this issue Nov 10, 2021 · 6 comments
Labels
library Relates to an Origami library

Comments

@cdaringe
Copy link
Contributor

cdaringe commented Nov 10, 2021

What

I need to build a targeted polyfill for browser X, but I do not know the polyfill-abe features that browser X needs.

  • e.g. provide a function f, where f(safari, 12) => ["ResizeObserver", "INTL.XYZ", ..., other-missing-features]

This is highly desirable functionality, because:

  • compiler toolchains may already compensate for some features, which may not need to be polyfilled, so those could be filtered out when generating polyfills. For example, f(safari, 12).filter(feature => !DONT_POLYFILL_THESE_HANDLED_FEATURES.some(rgx => rgx.test(feature))
  • a polyfill supporting features A & B & C may work with multilple browsers, so rather than going directly from browser => polyfill-bundle, instead, i can go from browser => features => polyfill-bundle and recycle my polyfill bundle between a few browsers. it is certainly useful to polyfill by features, not by browser target.
  • visualizing missing features can help me understand what sort of constraints i should put into my application development process and build pipeline

There is a related issue that says use polyfill-service-url-builder, but that library is more or less a function of src-js => polyfill-js, which is fundamentally disjoint with this issue. This is browser-identifier => list-of-features.

Details

  • explain what prompted this request — e.g. is it something that you regularly make a workaround for?

I have a few applications to manage. Each one has different browser support requirements, but I do not know the exact features I need to polyfill. I could, however, with just a tiny bit of extra information already contained in polyfill-library.

var Features_to_polyfill = 
   Features_used_by_app -  // known (user-space)
   Features_supported_by_compiler - // known (user-space)
   Features_not_in_browser // known, **but not exposed, by polyfill-library**
  • describe what the new feature would do and how it would be used
const pf = require('polyfill-library')
pf.getMissingFeatures(browser, version) // [list, of, features]
  • explain what alternatives you have explored / considered
  • where possible, attach designs for the style of the new feature

polyfill-library has all of this great data, it just does not expose it conveniently. it can be easily supported by this library, using existing APIs. However,

  • supporting this feature externally (that is, not from this library) requires knowledge about the meta.browsers schema, which I didn't see clearly documented. this is a risk in maintaining such functionality in user space.
  • mapping browserslist naming convetions => polyfill-library's meta.browsers schema may be challenging. hopefully contributors can articulate where the browsers TOML/meta schema convention is maintained? it would be awesome to unify that schema.

Here is an implementation. FWIW, in writing this, I believe I uncovered a few small bugs in the current library.

import assert from 'assert'
import * as pf from 'polyfill-library'

/**
 * Read & parse all polyfill-library meta.json files, indexed by 
 * feature name.
 * Forgive the cuteness, this is an expensive op, so i cached
 * the result
 */
export const getMetaByFeature = (() => {
  const metaByFeatureName: Record<string, pf.PolyfillMeta> = {}
  let cache: Promise<Record<string, pf.PolyfillMeta>> | null = null
  return () => {
    if (cache) return cache
    cache = Promise.resolve().then(async () => {
      const features = await pf.listAllPolyfills()
      await Promise.all(features.map(async feature => {
        const meta = await pf.describePolyfill(feature)
        if (meta) {
          metaByFeatureName[feature] = meta
        } else {
          // eslint-disable-next-line no-console
          console.error(`no meta for feature: ${meta}`)
        }
      }));
      return metaByFeatureName
    })
    return cache
  }
})()

export const getMissingFeatures = async (browserName: string, version: number, filter?: (feat: string) => boolean) => {
  const metaByFeatureName = await getMetaByFeature()
  const featuresToPolyfill = Object.entries(metaByFeatureName).filter(([feature, meta]) => {
    if (filter) {
      const isFiltered = filter(feature)
      if (isFiltered) return false
    }
    // @todo file a @types/* bug, but likely, this is polyfill-library bug for matched browsers
    // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
    if (!meta.browsers) {
      console.log(`meta missing for feature: ${feature}`)
      return false
    }
    const polyfillExprForBrowser = meta.browsers[browserName]
    if (!polyfillExprForBrowser) return false
    if (polyfillExprForBrowser.endsWith('*')) return true
    if (polyfillExprForBrowser.startsWith("<")) {
      // cases:
      // <1
      // <1.2.3
      // <=1.2.3
      // <= 1.2.3
      const [, lte, polyFillOnLessVersion] = polyfillExprForBrowser.match(/^<(=?)[^\d]*(\d+(\.?(.\d+)?)*)$/) || []
      assert.ok(!!polyFillOnLessVersion, `<VERSION match failed ${polyfillExprForBrowser}`)
      const rhs = parseInt(polyFillOnLessVersion, 10)
      const shouldPolyfill = lte ? version <= rhs : version < rhs
      return shouldPolyfill
    }
    const [, upperRangeMatch] = polyfillExprForBrowser.match(/.*-.*(\d+(\.?(.\d+)?)*)$/) || []
    if (upperRangeMatch) {
      return version <= parseInt(upperRangeMatch,10)
    }
    const [, exactVersionMatch] = polyfillExprForBrowser.match(/^(\d+(\.?(.\d+)?)*)$/) || [];
    if (exactVersionMatch) {
      return version < parseInt(exactVersionMatch,10)
    }
    if (polyfillExprForBrowser.startsWith('>')) {
      console.log(`bogus range missing for feature: ${feature} // ${polyfillExprForBrowser} // ${browserName}@${version}`)
      return false
    }
    throw new Error(`unhandled case ${polyfillExprForBrowser}`)
  }).map(([feature]) => feature)
  console.log(JSON.stringify(featuresToPolyfill, null, 2))
  return featuresToPolyfill
}

getMissingFeatures("safari", 12, feature => [
  /ESAbstract/,
  /ArrayIterator/,
  /~locale/
].some(rgx => rgx.test(feature)))


/**
Output:

meta missing for feature: AudioContext
bogus range missing for feature: HTMLInputElement.prototype.valueAsDate // >=10.1 // safari@12
bogus range missing for feature: Promise.prototype.finally // >7 <11.1 // safari@12
[
  "Element.prototype.inert",
  "Intl.DateTimeFormat",
  "Intl.DateTimeFormat.~timeZone.all",
  "Intl.DisplayNames",
  "Intl.DateTimeFormat.~timeZone.golden",
  "Intl.ListFormat",
  "Intl.Locale",
  "Intl.NumberFormat",
  "Intl.PluralRules",
  "Intl.RelativeTimeFormat",
  "MediaQueryList.prototype.addEventListener",
  "ResizeObserver",
  "String.prototype.replaceAll",
  "UserTiming",
  "WebAnimations",
  "_DOMTokenList",
  "_Iterator",
  "_StringIterator",
  "_mutation",
  "requestIdleCallback",
  "smoothscroll",
  "setImmediate",
  "screen.orientation"
]
*/

In executing the above script, I believe I have discovered a few small bugs in polyfill-library:

  • meta missing for feature: AudioContext
    • yep, no meta.json at all for this file!
  • bogus range missing for feature: HTMLInputElement.prototype.valueAsDate // >=10.1 // safari@12
    • I believe this says "polyfill for safari 10.1 and later", which I do not think is correct
  • bogus range missing for feature: Promise.prototype.finally // >7 <11.1 // safari@12
    • this should probably be just <11.1, not >7 && <11.1? unless some other polyfill is expected to capture it?
@github-actions github-actions bot added the library Relates to an Origami library label Nov 10, 2021
@cdaringe cdaringe changed the title RFC: support f(browser) => array-of-features RFC: support f(browser) => array-of-missing-features Nov 10, 2021
@romainmenke
Copy link
Collaborator

romainmenke commented Nov 10, 2021

Hi @cdaringe,

Thank you for the issue!

Some quick notes on the version info in config.toml files.

Ranges can define a minimum version required.
e.g. A polyfill that doesn't work in IE8 : ie = "9 - 11"


bogus range missing for feature: HTMLInputElement.prototype.valueAsDate // >=10.1 // safari@12
I believe this says "polyfill for safari 10.1 and later", which I do not think is correct

This exception is documented in the notes here : https://github.com/Financial-Times/polyfill-library/blob/master/polyfills/HTMLInputElement/prototype/valueAsDate/config.toml#L21

Safari older than 10.1 does not allow re-defining 'HTMLInputElement.prototype.valueAsDate'.


bogus range missing for feature: Promise.prototype.finally // >7 <11.1 // safari@12
this should probably be just <11.1, not >7 && <11.1? unless some other polyfill is expected to capture it?

https://github.com/Financial-Times/polyfill-library/blob/master/polyfills/Promise/prototype/finally/config.toml#L10

The Promise polyfill includes Promise.prototype.finally, to avoid needlessly serving this polyfill to those browsers, we have configured the browser targets for this polyfill to not include those configured in the Promise polyfill.


It is also not immediately clear to me what the end goal of getMissingFeatures would be?

Might be interesting :

https://github.com/mrhenry/core-web/blob/main/packages/core-web/index.ts#L37-L70

We use : semver.satisfies("<some browser version>", "some range from config.toml")

The goal on our end is to bundle polyfills from both core-js and polyfill-library without having conflicting duplicates. (multiple bundles are created for progressively older browsers)

It is a public function, so you can run this :

const { required } = require("@mrhenry/core-web");

console.log(required({
	browsers: { "safari": "12"}
}));
[
  'AbortController',
  'Element.prototype.inert',
  'HTMLInputElement.prototype.valueAsDate',
  'IntersectionObserver',
  'Intl.DateTimeFormat',
  'Intl.DateTimeFormat.~locale.af',
  'Intl.DateTimeFormat.~locale.af-NA',
  'Intl.DateTimeFormat.~locale.agq',
  'Intl.DateTimeFormat.~locale.ak',
  'Intl.DateTimeFormat.~locale.am',
  'Intl.DateTimeFormat.~locale.ar',
  'Intl.DateTimeFormat.~locale.ar-AE',
  'Intl.DateTimeFormat.~locale.ar-BH',
  ...

That list however is not useful to us on its own and it is very long.
We use it only to get automatic imports of needed polyfills through a babel plugin.

@cdaringe
Copy link
Contributor Author

cdaringe commented Nov 10, 2021

Hey @romainmenke

Thanks for the feedback. I've taken the opportunity to simplify my original problem statement at the top of the issue. I'm going to answer your questions, but out of order :)

... not immediately clear to me what the end goal of getMissingFeatures would be?

My ultimate objective is to support building an intelligent, targeted polyfill, programatically. Implementing getMissingFeatures is part of that puzzle.

Practically speaking, I need to solve the below equation for Features_to_polyfill. Without solving this equation, I cannot build a sensible polyfill automatically, with confidence.

var Features_to_polyfill = 
   Features_used_by_app -  // known (user-space)
   Features_supported_by_compiler - // known (user-space)
   Features_not_in_browser // **getMissingFeatures(browser, version)**

getMissingFeatures(browser, version) provides Features_not_in_browser, which is otherwise missing.

Does that make sense? I prevent some features from being used in my application, generally via things like lint rules. My compiler (babel) down-levels some language features, which I also know about. So, if those two sets of features are known, I want to be able say "hey, what leftover features that I am using need to be polyfilled?". As you noted, the list can be enormous! This is a positive trait--I want to consider each and every one of these entities, and either A) forbid the feature in my application, or B) include it in my polyfill set! Perhaps, Features_excluded_from_app get's derived from this value, and added into the above equation.

This helps me protect my users by not using unsupported features, or, helps by polyfilling only where needed. Without getMissingFeatures(), I cannot produce the list of features I want to polyfill reliably. This process has already helped me by forcing me to go consider what INTL APIs I will/will not need to support!

On your range notes, i'll defer conversation there. Thanks for that. I need to get a bit more free time and dwell on those

@cdaringe
Copy link
Contributor Author

const { required } = require("@mrhenry/core-web"); is exactly what i'm looking for :), thanks. Looks like I wrote a bunch of code to re-emulate that content.

I didn't see references to core-web in this project. Can you help clarify the relation between these two? Ultimately I was hoping that required was exposed in this package :)

@romainmenke
Copy link
Collaborator

I didn't see references to core-web in this project. Can you help clarify the relation between these two?

We (mrhenry) used to request polyfills from polyfill.io but had similar issues.

  • how to maintain correct polyfill bundles across ±100 different websites
  • frontend dev teams struggled with browser support goals (missing polyfills, duplicate polyfills, ...)

core-web was our answer to that. We needed a tool that would magically fix browser support.

So polyfill-library has no relation to core-web. But core-web uses polyfill-library and we try to give back as much as possible.


Ultimately I was hoping that required was exposed in this package :)

We did not submit a patch to include something like the required function here as it isn't useful in itself (the output is never the final product), is opinionated and the code is trivial.

Maintaining it on our end just made more sense for us.
This also avoids issues with different conventions for browser names. (see : #1117)

We know exactly how we will use the output and our implementation is written exactly for that. Including it here would need more thought so that it solves the right issue for the most people.

That does not mean that it or something similar couldn't be included here.


Practically speaking, I need to solve the below equation for Features_to_polyfill. Without solving this equation, I cannot build a sensible polyfill automatically, with confidence.

Your example could be rewritten as such :

// code validation problem (e.g. linting)
var Features_not_allowed = 
   Features_used_by_app -  // known (user-space)
   Features_supported_by_compiler - // known (user-space)
   Features_supported_natively_for_browser - // known (mdn/caniuse)
   Features_available_in_polyfillio_for_browser // getMissingFeatures(browser, version)
// bundling problem
var Features_to_polyfill = 
   exists_in_both(
      Features_used_by_app, // known (user-space)
      Features_available_in_polyfillio_for_browser // getMissingFeatures(browser, version)
   ) -
   Features_supported_by_compiler - // known (user-space)

The first item (making features not allowed) is not something we (mrhenry) do automatically. This is handled through review as an automatic tool can't detect progressive enhancement.

The second one is pretty much https://github.com/mrhenry/core-web

Issues we encountered and solved :

  1. not having something like getMissingFeatures
  2. reading each config.toml is too slow (>3000 file open's)
  3. exclude polyfills from polyfill-library if they exist in core-js
  4. exclude feature names and aliases like default, modernizr , HTMLCanvasElement.protoype.toBlob (note the missing t in protoype)
  5. map feature names and aliases to babel AST's for matching against source code
  6. ensure that each feature is gated (if statement with feature detection)
  7. ensure that each helper is exposed correctly (_ArrayIterator)

1, 2, 3 and 4 were solved as one by generating a single file with all config formatted and filtered for our use case.

So not having something like getMissingFeatures was never an issue for us as we had to resolve 2,3 and 4 anyway.

This is starting to sound like an ad for core-web and I definitely do not want to push you into using it. Just sounds like you are trying to do something similar and will encounter the same issues. So wanted to share a bit of what we did.

@romainmenke
Copy link
Collaborator

@cdaringe going over a few older issues

Is this still an open issue on your end?

Given the limited bandwidth we currently have for polyfill-library a feature like this might be easier to realise as a separate package.

@cdaringe
Copy link
Contributor Author

I’ll reopen if I find compelling new ideas :)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
library Relates to an Origami library
Projects
None yet
Development

No branches or pull requests

2 participants