-
Notifications
You must be signed in to change notification settings - Fork 14
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.
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
RFC 171 Remove JavaScript support for legacy browsers #171
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, no arguments from me on dropping IE11. Do wonder a lot about intersection of this and RFC 160
I do however wonder if this RFC is of too broad a scope by also introducing the migration to ES6 modular JS as there is a lot to think about there and the thing that most concerns me about this RFC is that it creates a really deep hole of work that would need to be done (most of my comments seem to be related to consequences of that). A suggestion I'd have is to have this RFC as a starting step to just drop IE11 from JS and not use JS modules but allow using functions IE does not support etc (I believe Whitehall has already done this), then once that is clear finish off the work on #160 to switch to modular JS?
|
||
## Proposal | ||
|
||
The proposal is to embrace modern JS so that legacy browsers do not download any JS but see the non-JS version of GOV.UK. This should still provide a usable experience of the site as the site is built using the principle of progressive enhancement. This will also reduce page weight for those browsers and allow us to embrace modern JS techniques, leading to further code improvements in future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you be able to clarify what level of support, if any, legacy browsers would have? I know it's referenced in "Upgrade, but ignore the needs of legacy browsers" but it would be good to have clarity as it's something that is quite ambiguous.
i.e. would we still test on IE11 (or earlier) despite not supporting JS on them? At what level of legacy (hopefully IE11) do we just not test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll update this, but I think we'll follow the Design System team's approach for Grade X browsers. We'd do some initial testing to ensure that JavaScript doesn't run on older browsers and that the site is usable, and from that point on stop testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'd be great ⭐
|
||
The proposal is to embrace modern JS so that legacy browsers do not download any JS but see the non-JS version of GOV.UK. This should still provide a usable experience of the site as the site is built using the principle of progressive enhancement. This will also reduce page weight for those browsers and allow us to embrace modern JS techniques, leading to further code improvements in future. | ||
|
||
We should implement ES modules initially, with further work done later to potentially use other modern JS techniques. This will be done on a problem solving basis rather than upgrading everything - we will identify issues and look at how modern JS could solve them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the idea to implement ES modules relate to: #160 ? Is that plan going to move forward at all or a near one created?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your point about implementing ES modules as part of this change is well made, and I think I'm going to reduce the scope of this RFC as suggested to just dropping support for IE11. Our team is looking at how to improve our JavaScript generally this quarter, and ES modules are something I believe will be useful for us, so we'll include #160 in our thinking for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good 👍 I think that'll reduce the decisions this RFC has to consider.
I'm very happy to help if you'd like any info more on the Rails/Rubygems side of things for the wider integration, there's lots of nuances.
alert('hi'); | ||
``` | ||
|
||
Legacy browsers will only download `application.js` and no other JS files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the plan here not to use any bundlers? or is that undefined? Similarly with our use of modern JS syntax would we be doing ant transpiling or sticking with just what evergreen browsers support?
Do legacy browsers still load the JS if it's script type="module" ? I did think the advantage of <script type="module">
was that it prevented legacy browsers - as seems a bit of a bummer if they get errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some legacy browsers may download the script type="module"
but will not execute it. There's no 'need' to use import
within the JavaScript file to prevent them from executing.
Bear in mind that imports from within the JavaScript file will not be discoverable by the preload scanner and the browser won't know they're needed until application.js
has been downloaded and parsed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The aim of this approach would be to minimise the amount of JS downloaded by legacy browsers. If we just switch all of our script tags to type module, I believe they'd still download it even though they don't do anything with it. That's fine, but ultimately it would be better if they downloaded as little as possible if they're not going to use it. Will clarify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, whatever happens you might have a total nightmare getting legacy browsers not to download and having good performance for modern browsers. For example if you had just a small JS file and imports, you'd still want those imports on the prefetch list so they could be downloaded early. (Edit: of course IE11 doesn't support preloading so this point is a little moot)
Since these are browsers that are no longer going to be supported and make up an extremely low proportion of users I'd suggest just taking an attitude of "not ideal but not worth fighting" - it's what 20-40kb, size of a small image?
|
||
```JavaScript | ||
// application.js | ||
import './module1.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we decided on whether we're going to go down a .js or .mjs route?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No decision on this (or discussion as far as I'm aware) but I don't think .mjs
offers anything particularly helpful? And if we were to rename all the files we'd have to update a lot of manifest files. Happy to do it if we need to, but I'm not currently suggesting we should.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, it sounds like this is no longer is part of the scope anyway with: #171 (comment)
Probably one to learn from Design System about given they adopted that approach, I get confused about where the JS ecosystem is with this convention. Manifest files will likely depend on the extent of bundling done - not sure if they're used in all the potential ways Rails can use JS.
|
||
The Design System team have updated `govuk-frontend` to use [ES modules in version 5](https://frontend.design-system.service.gov.uk/importing-css-assets-and-javascript/#javascript), released in 2023. [ES modules](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules) are not recognised by legacy browsers, notably Internet Explorer 11 and below. These browsers are no longer supported by Microsoft. We need to keep up to date with `govuk-frontend`, which means deciding how we approach JS support for IE11. The Design System has their own definitions for [browser support](https://frontend.design-system.service.gov.uk/browser-support/). | ||
|
||
The presence of any modern JS such as ES modules will cause errors for older browsers. This is because they will attempt to parse all JS before running it, encounter unfamiliar syntax, and error. No JS will run on older browsers once any modern JS is introduced, which means that any download of JS will be wasted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to conflict with the line a bit later that application.js would be downloaded (unless I've misunderstood)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that all browsers will download application.js, so I guess technically by running that they are running some js. However, the code in application.js is safe enough to not error, but also safe enough to not lead to any further browser action in legacy browsers. Therefore effectively legacy isn't downloading or running any js.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. My understanding (which I had to just check with @richardTowers as Google was failing me) is that.
If you have <script type="module" src="application.js">
then old browsers don't understand type="module"
and thus do not attempt to download or execute the JS.
Edit: @36degrees clarified above: #171 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will reword for clarity.
My understanding - and it may be wrong! - is that using functions that way would mean the experience for IE (and other legacy) would degrade as it would try to run code that would bomb in the browser. Moving to ES6 syntax means legacy browsers don't understand how to load the js in the first place, so don't - a bit like the old tricks of using comment syntax to load browser specific code. The question you asked elsewhere around what the experience for such browsers would be is that they get the unenhanced no-js experience, with no js available at all. I think our testing strategy during build should be to check in IE that the experience is usable but probably doesn't need to exhaustively check other legacy once we're set up. Using new functionality gets greyer and the design system team are thinking about this now because you've got a tiered experience of shiny new browsers vs slightly older ones and need to consider the support levels and what is safe to use. The work on RFC160 stalled due to other priorities and some uncertainty - we need to get to the bottom of the detail but that's likely to be the implementation path we go down. |
Ah I didn't mean to just have the code error on it. More to use the trick to turn on modular JS to only allow evergreen browsers but not to jump headlong into everything being modules and thus needing to rewrite everything as part of the launch. That's the approach taken in Whitehall, could be worth chatting to @dnkrj about it. Would we be able to get that browser strategy documented? I'd love to know what browsers we're supposed to check are usable and which are totally ignored. Edit: I've checked the Whitehall code and all browsers load the JS and turns off JS enabled with: https://github.com/alphagov/whitehall/blob/bf678254b8bdb3930735f569f065c1985b668e1c/app/assets/javascripts/admin/stop-scripts-nomodule.js#L1-L15 I'm guessing that because of this it means old browsers will just error on syntax after that 🤔 |
I think the logical approach is to align with the design system on this: We sort of suggest we already do by linking out from here: https://docs.publishing.service.gov.uk/manual/browser-support.html |
That'd be great. By that we'd have IE11 as grade X so we have no commitment to check it and won't fix any bugs? |
I think I'd be uncomfortable with blanket refusing to fix bugs depending on the impact of said bug. And while there'd be no commitment to testing in it directly it's a good check for the no-js experience. |
Sure this makes sense but these are the sort of nuances that we struggle with as we end up with individual strategies beyond the design system, hence why I think we need more than design system documentation since we subtly vary from their rules. |
- reduce scope to not use ES modules - clarify browser support
Thanks for all the comments everyone. I've made some changes, notably reducing the scope as suggested to not implement modules and only prevent JS from running on legacy browsers with the use of |
Sounds great to me 👍 Thanks for all your work on this. (by the way the rendered version link is out of date if you set it to: https://github.com/alphagov/govuk-rfcs/blob/rfc-168/rfc-171-remove-legacy-browser-js-support.md it'll stay in sync with your edits) Couple of implementation questions, that I understand we may not have the answer to yet and aren't blocking for this RFC, so could just be future considerations:
|
Thanks @kevindew (especially for that link, that's brilliant). Good questions, here's some thoughts that I might end up adding to the RFC.
|
Thanks. Re:
I assume we can just do |
Oh yes of course 🤦 😁 |
Given this hasn't been in a state of change for a couple of weeks is it time to invoke the process and get a deadline on this so we can get this merged as agreed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me! I have one comment about strict mode, which I think you should consider, but I don't think it changes the fact that we should do this.
Once legacy browsers stop running our JS we can look at what actual modern JS we could use on GOV.UK, and ways of reducing the amount of JS downloaded by legacy browsers. We can also remove any polyfills currently included to support legacy browsers. | ||
|
||
We should also adopt the Design System's recommendations for [browser support](https://frontend.design-system.service.gov.uk/browser-support/). Legacy browsers would be tested initially to ensure that they do not execute JS, then treated as Grade X browsers as recommended. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One additional thing to be aware of here is that scripts loaded with <script type=module ...
implicitly have strict mode enabled.
I think we're inconsistent in our use of strict mode currently - some modules set it in function scope, but many others don't.
That means this RFC also effectively rolls out strict mode everywhere, which is a good thing, but we'll have to consider whether there might be any risks of breakages while transitioning to strict mode.
We might want to consider whether to roll out strict mode module by module first, or whether to just check everything and do it all at once by setting type=module
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... I nerd sniped myself with this a bit... here's a draft PR which introduces linting for 'use strict' in the components gem:
https://github.com/alphagov/govuk_publishing_components/pull/4020/files
If you look at the lint results, there are a lot of places where we don't set it.
On the other hand, I'm pretty confident it's a moot point, because the linter would probably complain if we did anything that's not allowed in strict mode. So I think I'd be okay with just going for it and switching to type=module and getting strict mode everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great 👍
Not a comment that is likely to affect this RFC but I wanted to flag an implementation issue we've experienced in GOV.UK Chat with exploring A difference between This issue is present in the GOV.UK Frontend code prior to v5 where the modules are initialised with a iife using this as the global variable: https://github.com/alphagov/govuk-frontend/blob/f706252d274f5c46e9ae8dc8d81d021aa4af45c9/package/govuk/components/skip-link/skip-link.js#L1-L5 as they error because However this is not an issue with GOV.UK Frontend v5 with the bundled modules as they now make use of globalThis: https://cdn.jsdelivr.net/npm/govuk-frontend@5.0.0/dist/govuk/components/skip-link/skip-link.bundle.js
|
|
||
While many of our components would rely on ES modules JS from `govuk-frontend`, a lot of our other code does not, and could in theory still work. This would mean legacy browsers would experience some but not all of the JS enhancement of the site. | ||
|
||
While this might be possible, the developer burden of maintaining these two parts of the JS would be unsustainable, for little reward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I think this also makes it difficult to communicate what users can expect to function as before by having partial/mixed support for IE11 (Grade X Browsers)
Thanks @kevindew. This might explain why a tech spike into doing this with our current version (v4) was unsuccessful, but attempting the same with v5 worked. Good to know. |
|
||
The proposal is to modify GOV.UK so that legacy browsers do not run any JS on GOV.UK. This should still provide a usable experience as the site is built using the principle of progressive enhancement. This will ultimately reduce page weight for those browsers and allow us to embrace modern JS techniques, leading to further code improvements in future. | ||
|
||
This will be achieved by switching our JS script tags to include `type="module"`. This is used in modern browsers to differentiate JS containing ES modules. Legacy browsers do not recognise this attribute and may download the JS but will not attempt to parse or run it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, a couple of other things for us to consider when it comes to implementation:
- We will likely want to follow the same approach used in v5 of govuk-frontend in our own modules as well - Add class
.govuk-frontend-supported
for ES modules support govuk-frontend#3801 - By default, when
type=module
attribute is added to the script tag, the script will automatically be deferred, for example, the scripts below run without waiting for any other events to fire, in this case the scripts just log the application name to the console.
Not using type=module
<script src="/assets/static/application.js"></script>
<script src="/assets/frontend/application.js"></script>
Console output
"static"
"frontend"
Using type=module
<script src="/assets/static/application.js" type="module"></script>
<script src="/assets/frontend/application.js"></script>
Console output
"frontend"
"static"
|
||
Once legacy browsers stop running our JS we can look at what actual modern JS we could use on GOV.UK, and ways of reducing the amount of JS downloaded by legacy browsers. We can also remove any polyfills currently included to support legacy browsers. | ||
|
||
We should also adopt the Design System's recommendations for [browser support](https://frontend.design-system.service.gov.uk/browser-support/). Legacy browsers would be tested initially to ensure that they do not execute JS, then treated as Grade X browsers as recommended. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, makes sense to follow this approach to browser support. I imagine we may want to document our approach to browser support as well, making reference to the browser support recommendations from the design system and adding any further exceptions we have made where required.
RFC 171 Remove JavaScript support for legacy browsers
Deadline for comments: 16th May 2024
Rendered version
Trello card: https://trello.com/c/G2oRQ3qx/37-write-rfc-for-dropping-older-browser-support