Review polyfill approach to writing progressively-enhanced, x-browser Javascript

adamsilver commented 6 years ago

I heard the project is considering a polyfill approach to JavaScript cross-browser support.

I spoke to Nick Colley on Slack about the disadvantages of polyfills and how we might want to consider a more robust approach moving forwards.

The approach is discussed in (my article) Progressively Enhanced JavaScript, but I learnt this from Peter Michaux’s Cross Browser Widgets article (don’t let the date put you off; the best techniques usually stand the test of time).

I’m happy to discuss/help further with this, but just getting this down as an issue for now.

NickColley commented 6 years ago

We're currently in a halfway place between this approach and a strict polyfilling approach.

You can see the details component uses the approach described here: https://github.com/alphagov/govuk-frontend/blob/master/src/details/details.js#L11

Whereas the Checkbox component uses the (if we continue) the new approach: https://github.com/alphagov/govuk-frontend/blob/master/src/checkboxes/checkboxes.js

adamsilver commented 6 years ago

It seems you've already got a lot of the code necessary written to create a wrapper interface as opposed to polyfills. So with a little bit of work, you could avoid the polyfill problem.

NickColley commented 6 years ago

They augmenting host objects

This is a valid point but seems to be being made mainly on the basis of extending functionality that is not part of the stable ECMAScript specification. Which is not what we're doing.

Feature detection is not enough

Well reasoned and makes sense, however the detections used by the Financial Times account for this. Where necessary they do more than check the prototype of a global feature. Even in a utility we'd want to do feature detection, so it's just a matter of where this is done.

They intertwine browser and application logic

This argument seems to be made without considering polyfills, I agree that it's a bad idea to expose your application logic to complexities with different browser but both utilities and polyfilling achieves the same goal here?

We rarely need need the full API

This makes sense.

They come with caveats

This is definitely a risk.

Another risk is that we cannot be sure our polyfills do not break third party code.

I think the biggest positive with polyfilling is we are writing JavaScript that does not require developers to understand our abstractions. If you understand how events work in JavaScript addEventListener will just work.

Final thing to note is that while putting trust in the FT team, they're using this service across many projects. https://polyfill.io/v2/docs/

adamsilver commented 6 years ago

Thanks @nickcolley

This is a valid point but seems to be being made mainly on the basis of extending functionality that is not part of the stable ECMAScript specification. Which is not what we're doing.

What do you mean by stable here? Every host object API is subject to the pitfalls of augmentation.

the detections used by the Financial Times account for this. Where necessary they do more than check the prototype of a global feature.

Agreed—I should amend my article. It's not a fault of polyfills per se. Thanks

This argument seems to be made without considering polyfills, I agree that it's a bad idea to expose your application logic to complexities with different browser but both utilities and polyfilling achieves the same goal here?

Agreed.

NickColley commented 6 years ago

What do you mean by stable here? Every host object API is subject to the pitfalls of augmentation.

My interpretation is that the biggest issue with modifying native APIs is when it is done with no regard to standards. For example when MooTools used 'flatten' (https://github.com/tc39/proposal-flatMap/pull/56).

When polyfilling stable and accepted standards, this risk is not there? (With point five around cavets still in mind)

adamsilver commented 6 years ago

My interpretation is that the biggest issue with modifying native APIs is when it is done with no regard to standards. For example when MooTools used 'flatten' (tc39/proposal-flatMap#56).

The naming issue is still an issue, although it's not the most pressing.

It's important to note that host objects (as opposed to native built-ins) are far more dangerous. So el.addEventListener is far more dangerous than say Array.prototype.filter.

The other issues are documented here:

http://perfectionkills.com/whats-wrong-with-extending-the-dom/#lack_of_specification
http://perfectionkills.com/whats-wrong-with-extending-the-dom/#host_objects_have_no_rules
http://perfectionkills.com/whats-wrong-with-extending-the-dom/#performance_overhead
http://perfectionkills.com/whats-wrong-with-extending-the-dom/#ie_dom_is_a_mess (admittedly, that browser is probably not on our radar anymore, but see the point about host objects having no rules)

NickColley commented 6 years ago

@adamsilver it seems to be talking about 'extension', which I take to mean: Additional features that are not part of the spec (for example, flattern). Polyfilling does not extend DOM in the same respect, but rather ensures the spec standard is implemented, is this your understanding too?

adamsilver commented 6 years ago

Extension and polyfills are exactly the same thing here.

You take a browser that doesn't have an API, let's say el.addEventListener. Then you extend el (or objects of that type such as HTMLElement) with a user-defined addEventListener method.

NickColley commented 6 years ago

I've spent some time reading the linked articles in detail.

I'm struggling to find real world evidence that show these issues applied to polyfilling, what I have found seem to be reflecting on Prototype/MooTools abuse of native APIs.

The author's follow up article includes arguments that seem to be in favour of polyfilling, with only exception being 'host objects'.

Since GOV.UK Frontend is expected to be used in places we cannot anticipate maybe we should err on the side of caution.

This leaves us a few options:

1. Utility approach only

Pros:

No chance of conflicts with third party scripts.

Cons:

Utilities are an abstraction that developers need to learn.
There is no single third party source that we can easily use, so we would have to write these ourselves (or contributors would), and/or consider projects like lodash.
Not using JavaScript APIs, may make code such as when we use bind cumbersome to author.

(Note: One argument could be to continue using jQuery, but 1.x is no longer maintained and newer versions do not have the browser support we require)

2. Continue with Polyfills

Pros:

Very easy to remove in the future
JavaScript written is standard and transferable.
Approach can be easily maintained and used by the community.
Cons:
Possibility of bugs when GOV.UK Frontend is loaded with JavaScript we can't test with.

3. Only polyfill ES5 features, using utilities instead of DOM polyfills.

A compromise between 1. and 2., but would satisfy the issues, so could be an option. This is essentially what we're doing in 'govuk_frontend_toolkit' where we polyfill bind but use jQuery as as a utility abstraction.

Interested to hear what the rest of the community / team thinks. Thanks @adamsilver for bearing with me 😄

frankieroberto commented 6 years ago

I'm quite pro polyfills, mainly on the basis that they help developers to understand the native API rather than having to learn an abstraction. This is especially important with organisations such as government which have highly aggregated and unstable teams (eg with lots of contractors coming and going, and employees moving around lots).

The ability to easily drop polyfills in future is another bonus.

That said, I think it does put an onus on the govuk team to promote the 'best' way to do feature detection for each individual feature, that works across browsers and with both the native and polyfilled implementations.

There are also some features (such as the Drag and Drop API) which are impossible to feature-detect accurately without browser sniffing (blame mobile Safari, see https://stackoverflow.com/questions/2856262/detecting-html5-drag-and-drop-support-in-javascript).

adamsilver commented 6 years ago

I think we need to be careful about convenience for devs vs stability and resilience for users.

Although, I do think that polyfills are actually a burden on developers too.

I agree that documentation would be needed, but not sure that's a bad thing. Do the hard work to make it simple. :)

NickColley commented 6 years ago

Update: I have proposed a piece of work to spike this proposal and see if it's possible, which we'll do within our current sprint.

https://trello.com/c/Bef9APKQ/959-spike-swapping-polyfill-approach-with-utilities

pauldwaite commented 6 years ago

@adamsilver

I think we need to be careful about convenience for devs vs stability and resilience for users.

Absolutely. But plenty of polyfills don’t affect stability and resilience for users.

A utilities library can affect stability and resilience for users too, if it’s not written, tested and maintained properly, or if it’s not understood by developers. And as the application code would rely on the utilities library, that maintenance and understanding work will be ongoing, whereas polyfills done the right way will eventually be deleted.

adamsilver commented 6 years ago

@pauldwaite I've not yet had the chance to fully present what the alternative to polyfills would be. I apologise for that as the onus is on me.

If I had, I'd hope to show you that actually, a library is less work and can just as easily be ripped out in the future too.

Bare in mind that with polyfills, if you rip them out, then the interface is likely to break ; so you'd have to be able to either:

guarantee that no users ever will use a browser that lacks support for said method/property/feature; or
simply not care that one or more users experienced that breakage.

These are two scenarios that as devs/designers we wouldn't have to ever think about again, given what I'm proposing.

A utilities library can affect stability and resilience for users too, if it’s not written, tested and maintained properly, or if it’s not understood by developers.

Yes, I felt as though this was a given, but you're right on with this.

pauldwaite commented 6 years ago

@adamsilver I get your point about the consequences of removing polyfills. I think we already don’t care than one or more users experience breakage: the service I’m working on will definitely break in IE 6 and 7, and I think we’re fine with that.

I’d be interested in the “less work” comparison. I’ve mainly grabbed already-written polyfills, mostly from MDN. The work has been pretty minimal, if you ignore the other people’s work in writing the polyfills in the first place.

frankieroberto commented 6 years ago

@adamsilver providing that javascript is only ever progressive enhancement, and that feature-detection is used, removing the polyfill doesn’t break things, it just means that those users get the non-javascript experience.

leekowalkowski-hmrc commented 6 years ago

I've had a quick skim of the arguments against polyfills (I'm pressed for time), but I didn't see any convincing reason to avoid them.

If I understand it right, the argument is that a polyfill is considered to be like a parasitic invasion of a host object. Which in the strict context of the host, is accurate, but in the context of developing to a standard platform, then that's not a helpful analogy.

A polyfill is more like a cure than a disease, as long as:

It does what the real thing does, and nothing more (which is unfortunately often not trivial for HTML element polyfills).
It only does it when necessary (i.e. is redundant if the native feature is available).
It results in a developer (or user) experience identical to having the real thing. (Or the fallback experience for the user is acceptable)

If it's not possible to create a polyfill without such side effects, then I would probably decide the feature is too cutting edge and postpone it (no polyfill, no library, no anything until later).

adamsilver commented 6 years ago

Polyfills extend native built-ins and host objects. The main problems are with host objects.

For some examples of problems read the entire section here:

http://perfectionkills.com/whats-wrong-with-extending-the-dom/#host_objects_have_no_rules

That's a good start to understand the problem with messing with host objects.

adamsilver commented 6 years ago

Here's a related problem. Take document.querySelectorAll().

Today, it's support is broad. However, in browsers that don't support it, our scripts (without a polyfill) will throw and likely break the experience. Something I described in the article I mentioned above.

The polyfill approach means we have to either let things break for users (or hope that nobody uses those browsers that lack support), or we have to write a polyfill. That's a lot of fuss and a lot of code (that's what jQuery's Sizzle is—big and buggy).

If, however, we didn't use the polyfill approach, we could do something like this:

https://github.com/cinsoft/jessie/blob/master/src/functions/query/rendition1.js

(that's one way to solve it)

Then we don't need a polyfill at all and it either enhances in browsers that support it, or it doesn't in those that don't (like JS off).

The amount of code there isn't an illusion. A ton of the code we'd have to write is tiny.

With polyfills we're overly constrained as per the above example.

That's just one API example. We have to use all sorts of APIs in combination. So this problem scales badly.

adamsilver commented 6 years ago

Related.

We can polyfill relatively easily el.addEventListener() except el.attachEvent() isn't exactly the same. And in any case neither of these will let us do event delegation.

The point here is that we're going to need another abstraction (read utility/helper/libary functions) on top of the native APIs anyway.

frankieroberto commented 6 years ago

@adamsilver I don't think there are any available polyfills for querySelectorAll (especially given that the CSS selector syntax is so large and ever-expanding), so I suspect that for that API, the only question is about how to feature-detect it (so that older browsers don't throw a javascript error)?

Which means either:

if ('querySelectorAll' in navigator) {
   var links = document.querySelectorAll('main a');
}

or an abstraction like:


if ('querySelectorAll' in navigator) { 
   function query(selector) {
      return document.querySelectorAll(selector);
   }
}

if (query) { 
   var links = query('main a'); }
}

The former has the advantage (in my view) of exposing developers to the actual native API method name, whereas the latter has the advantage of developers not having to think about the best way to do the feature detection (or forgetting to do it).

adamsilver commented 6 years ago

Thanks @frankieroberto

It's 'querySelectorAll' in window (or better yet use isHostMethod(window, 'querySelectorAll')) :)

See: http://peter.michaux.ca/articles/feature-detection-state-of-the-art-browser-scripting

Questions:

should all developers/designers who are choosing to use / relying on a library have/want/need to learn the native/host APIs?
if they do want to learn the APIs (and that's a big if imo), can't they just look under the hood as opposed to be forced to be exposed to it?
And let's say that all developers should be exposed to the native APIs because “we” think that it's best for them to know. Isn't it best for us to show them how to build a robust, feature-detected library of functions :)

pauldwaite commented 6 years ago

Question: should all developers/designers who are choosing to use / relying on a library have/want/need to learn the native/host APIs?

The native/host APIs are always there. Libraries on top of them come and go.

frankieroberto commented 6 years ago

@adamsilver the problem with libraries are that there's so many of them! The chances of every government department agreeing to use the same ones seems slim, let alone the wider sector (and it's worth bearing in mind that many of the designers/developers working on government services are contractors or on secondment).

Learning the native API is always useful, in my view. We learnt that the hard way with jQuery... ;-)

I suspect the bigger issue though will be that, as APIs like addEventListener become increasingly known and used (especially by those who don't have to worry so much by older browsers), it’s much easier for developers not to have to learn a library.

That said, I acknowledge the downside of having to teach/show people the best ways to do feature detection.

pauldwaite commented 6 years ago

In terms of feature detection, there is also the BBC’s “cutting the mustard” approach. (I like to think this is named after an obscure 70s sitcom.)

As opposed to putting a (fairly thin) library on top of the native APIs (as I think @adamsilver is suggesting), they essentially do one-liner feature detection for a few key features at the start of their script, and give up if any of the features aren’t present.

It’s a bit blunt, and probably easy to miss when reading existing code, compared to the library approach. But it is fairly isolated from the rest of the code.

adamsilver commented 6 years ago

@pauldwaite @frankieroberto

Granted this might be hard to believe, but a well-designed library of cross-browser progressively enhanced JavaScript (woah, what a mouthful that was) can stand the test of time.

Re: Cutting The Mustard The idea of CTM is great, the execution falls a little short: https://adamsilver.io/articles/progressively-enhanced-javascript/#dont-rely-on-cutting-the-mustard

adamsilver commented 6 years ago

The solution I'm poorly trying to communicate is that we could end up writing JS a bit like this:

if(govuk.hasFeatures('addEventListener', 'querySelectorAll', 'isArray')) {

  // define a component that uses these methods such as a tab component

  function Tab(blah) {
    var tabs = govuk.querySelectorAll('.blah');
    govuk.addEventListener(tabs, 'click', ...);
  }

  Tab.prototype.blah = function() {
  }

}

This is just an example API.

leekowalkowski-hmrc commented 6 years ago

They are confusing reasons against polyfills.

I didn't understand the how the approach described for document.querySelectorAll was actually useful? So instead of using querySelectorAll, I call a new function called query, which doesn't have a solution within it because it relies on querySelectorAll? I feel like I missed the point. I certainly wouldn't use the alternative query because it's not apparent what the benefit is. It appears to be a needless wrapper.

With attachEvent, if it does not do event delegation, then polyfill or otherwise, you cannot use event delegation. No? So I didn't understand how event delegation itself is a concern that influences whether or not you should polyfill.

I can only make sense of this as "there are some situations where polyfilling is a no-go, therefore, never use polyfills for anything". Which feels to me like throwing the baby out with the bath water.

My approach to adopting semi-supported technology on public websites is that if it cannot easily be polyfilled (e.g. if one particular browser makes it impossible: 'Trying to overwrite "target" property of event object in Mozilla throws TypeError'), testing should be sufficient to discover this is the case, and should result in the decision to avoid the semi-supported technology. It's often a nice-to-have and not driven by the users' needs, anyway.

This does mean I'm often working with feature set which of the lowest common denominator available natively, incidentally. Which means I don't create polyfills lightly. My commercial sites only have Array.from and Array.indexOf as polyfills. I tend to use the ancient element.onclick style event handlers because I can design away any conflicts since I don't have the misfortune of dependencies on 3rd party libraries - modern native capability is good enough for everything.

I can appreciate the arguments against polyfills that are too ambitious or if the target audience is everybody, everywhere - the polyfill may fail in unexpected ways, interfere with other things, or not satisfy a niche case. I think this comes from the desperation to use a new feature for the sake of it, rather than the fact we chose to polyfill it.

For example, writing a custom 'reveal password' feature, polyfill or not, is doomed to be surpassed by native implementations over time irrespective how good a job was made of it.

pauldwaite commented 6 years ago

I can definitely see the benefit of a feature-detection library (like govuk.hasFeatures).

What I’m a bit leery about is a library that provides an API on top of those features (govuk.querySelectorAll).

(And, to be clear, that’s just my instinct. Definitely a great candidate for some user research with developers.)

adamsilver commented 6 years ago

I’d be interested in the “less work” comparison. I’ve mainly grabbed already-written polyfills, mostly from MDN. The work has been pretty minimal, if you ignore the other people’s work in writing the polyfills in the first place.

If you ignore the work of writing the polyfill, then you ignore the work of writing the alternative to polyfills.

adamsilver commented 6 years ago

@leekowalkowski-hmrc

I didn't understand the how the approach described for document.querySelectorAll was actually useful? So instead of using querySelectorAll, I call a new function called query, which doesn't have a solution within it because it relies on querySelectorAll? I feel like I missed the point.

You either missed the point, or I failed to explain (probably the latter).

With the wrapper code in place you can now do this:

if(govuk.hasFeatures('query')) {
  // the browser supports govuk.query
  // (which we know under the hood supports querySelectorAll)
  // so now we can enhance the interface using that api
  // this is what progressive enhancement is
}

It's worth reading the articles I cited above in full, as opposed to skimming them. Here's a good one:

http://peter.michaux.ca/articles/feature-detection-state-of-the-art-browser-scripting

adamsilver commented 6 years ago

@leekowalkowski-hmrc

So I didn't understand how event delegation itself is a concern that influences whether or not you should polyfill.

Because part of the temptation to want to use polyfills stems for the fact that you can just interface directly with host objects (which is ideal if it worked). Where as actually, to be able to delegate events (and that's just one of a million examples) you'd need an abstraction anyway.

adamsilver commented 6 years ago

@leekowalkowski-hmrc

My approach to adopting semi-supported technology on public websites is that if it cannot easily be polyfilled

That's a real unfortunate approach you're taking there. For example, geo location. You can't polyfill that. But with a simple feature detected wrapper (see above etc etc etc) then you can enhance for browsers that support it without hurting users of browsers that don't.

pauldwaite commented 6 years ago

@adamsilver

If you ignore the work of writing the polyfill, then you ignore the work of writing the alternative to polyfills.

If the work of writing the alternative has already been done by other people, sure, ignore it. My understanding of this proposal is for people working on GOV.UK code to do that work.

adamsilver commented 6 years ago

@pauldwaite

If the work of writing the alternative has already been done by other people, sure, ignore it. My understanding of this proposal is for people working on GOV.UK code to do that work.

Yes, but alternatively, there would need to be work to use/test/port/integrate/write/document polyfills GOVUK front end.

joelanman commented 6 years ago

I think its good to acknowledge there would be more work, that has to be part of the discussion, even if it's worth doing the work.

NickColley commented 6 years ago

Hey everyone,

Thanks so much for the conversation, it gives us a good basis to make a decision on.

Will be making a decision on this next sprint. 👍

leekowalkowski-hmrc commented 6 years ago

With the wrapper code in place you can now do this: if(govuk.hasFeatures('query'))

But you could already do if(document.querySelectorAll) which is what I would prefer*, especially when query is an arbitrary synonym that would have to be learned, having to discover these associations could be frustrating.

Unless I decide that querySelector is within my 'lowest-common-denominator', which these days, I do. In the days when I did not, I avoided querySelector altogether.

This is not one of the situations the feature detection article warned about (because there is always a document object available, you can do it as early as you need to).

That's a real unfortunate approach you're taking there. For example, geo location.

Ah, no, sorry, I'm not saying I avoid the technology completely in these situations, we can absolutely embrace features for the users that have it via progressive enhancement.

What I don't like is when we try to create an equivalent experience for users that lack such features, especially because in my experience as an end user, the 'equivalent' I am given is inferior at best, broken at worst, and just frustrating when I know a simple alternative technique exists that just always works (e.g. enter a date as a single field instead of being forced to interact with a flaky date picker).

should all developers/designers who are choosing to use / relying on a library have/want/need to learn the native/host APIs?

No, that's the point of using the library.

if they do want to learn the APIs (and that's a big if imo), can't they just look under the hood as opposed to be forced to be exposed to it?

They could, except many libraries are over-complicated and almost impossible to read. I would say they're not the easiest source for learning.

And let's say that all developers should be exposed to the native APIs because “we” think that it's best for them to know. Isn't it best for us to show them how to build a robust, feature-detected library of functions :)

Indeed. As long as we don't do potentially confusing things like rename 'querySelector' to 'query'.

When I was in the HTML Working Group, I remember we were occasionally reminded about the priority of constituences.

Basically the priority is the needs of: users > web developers > browser developers > W3C > technical purity.

The argument that we always avoid polyfills because they manipulate host objects, which is sometimes dangerous, sounds to me like making an argument for technical purity. Which means we need to consider if that is going to have a knock-on cost for developers or users.

An example of this is BEM vs CSS with an actual cascade, BEM makes a CSS author's life easier, but at the cost of the HTML author, for whom it is a cognitive nightmare.

adamsilver commented 6 years ago

The argument that we always avoid polyfills because they manipulate host objects, which is sometimes dangerous, sounds to me like making an argument for technical purity. Which means we need to consider if that is going to have a knock-on cost for developers or users.

Yep, agree, that's a more than valid concern. For me it's not about technical purity but robustness for users. But again, really valid and good to add to the discussion.

As long as we don't do potentially confusing things like rename 'querySelector' to 'query'.

Yep, the name isn't ideal. The name could totally be changed.

frankieroberto commented 6 years ago

An example of this is BEM vs CSS with an actual cascade, BEM makes a CSS author's life easier, but at the cost of the HTML author, for whom it is a cognitive nightmare.

Ha, yes, agree with this 100%! All those long abbreviated class names!

alex-ju commented 6 years ago

The GOV.UK Design System team had a meeting discussing the proposal and carefully considered all the points raised in this issue.

We have decided to continue with the polyfill approach.

We believe that the community around polyfills is more mature and will help others building components outside of GOV.UK Frontend without a reliance on our team to maintain a utility library.

We accept and understand the risk around the extension of native APIs, given that we are intending to use polyfills that are aiming to be spec compliant.

In cases where polyfills are not yet available or would result in poor behaviour, we will consider ‘cutting the mustard’ (as can be seen in the Accessible Autocomplete project) or using utility functions.

Thanks everyone for contributing.

alphagov / govuk-frontend