alphagov / tech-docs-gem

Gem to distribute the tech docs project
https://tdt-documentation.london.cloudapps.digital/
MIT License
15 stars 38 forks source link

Upgrading to govuk-frontend v5 #352

Closed kr8n3r closed 1 month ago

kr8n3r commented 2 months ago

As part of maintenance and WCAG 2.2 compliance, we'd like to use the latest major version of govuk-frontend Version 5 Javascript support is limited to this support grades https://github.com/alphagov/govuk-frontend/blob/main/docs/contributing/browser-support.md as they use ES modules "Only browsers in grades A, B and C will run our JavaScript enhancements. We will not support our JavaScript enhancements for older browsers in grade X"

We need to decide if a) we're going to use babel to transpile to ES5 or b) build govuk-frontend Javascript as intended and serve a no-js experience for browsers that don't support it.

Most of the Javascript modules here are ES5 using global window so we'll need to think about potentially rewriting those to ES modules.

Additionally, downstream projects can write and include their own additions to application.js. We can expect that their javascript is likely to be ES5 and possibly using jQuery as well.

One of the options is to generate two files: ESM and non-ESM and bundle downstream into the latter. Similar to what we're doing on notify-admin for now

huwd commented 2 months ago

A concern I have is us changing the asset pipeline in a way which impacts any JS currently written in various tech docs sites around.

If we needed for force a rewrite there is dramatically raises the cost of this as a breaking change. Any thoughts there on if we could bundle both ESM and non-ESM and then ensure that any existing JS in the doc site remains compatible during build?

If we did that then this might not even be a breaking change?

kr8n3r commented 2 months ago

A concern I have is us changing the asset pipeline in a way which impacts any JS currently written in various tech docs sites around.

If we needed for force a rewrite there is dramatically raises the cost of this as a breaking change. Any thoughts there on if we could bundle both ESM and non-ESM and then ensure that any existing JS in the doc site remains compatible during build?

If we did that then this might not even be a breaking change?

yes do "generate two files: ESM and non-ESM and bundle downstream into the latter"

then anyone who currently adds their own javascript into the application.js like https://github.com/alphagov/govwifi-tech-docs/blob/master/source/javascripts/application.js would be able to do so. if we need to use any govuk-frontend JS in the gem, then we'd create an ESM bundle

huwd commented 2 months ago

oops forgot to push this earlier

I've started a branch here: https://github.com/alphagov/tech-docs-gem/tree/test-upgrade-to-5x

Tried to address all the initial build issues, but will need some help thinking through the JS from here on out. Hopefully a useful starting point if we want to pair / work on it next week?

kr8n3r commented 2 months ago

I'll have a look at your branch and try approaches there

romaricpascal commented 2 months ago

If that's helpful for the update, GOV.UK Frontend also provides unminified UMD modules. While they're not the main route for using GOV.UK Frontend, we have them in for projects whose assets compilation do not necessarily support ESM.

The file with all the components is all.bundle.js, though if the tech_docs_gem only use specific components, best to only import those individually (each have a <COMPONENT_NAME>/<COMPONENT_NAME>.bundle.js file) to reduce the final bundle size.

We'd still recommend loading the built script with <script type="module">. This is to avoid older browsers loading scripts that they won't be able to parse as these UMD modules have modern syntax in them (const, let, class...) that older browsers will error on.

kr8n3r commented 2 months ago

@huwd something along the lines of https://github.com/alphagov/tech-docs-gem/commit/39502c0c628456cf0780174987a2723fce72d0a9 separate entry for govuk-frontend with type module (either minified file or all.mjs and we compile it with the new asset pipeline)

huwd commented 2 months ago

Ah great, I'll take a closer look at that this week - perhaps we can find that time to pair.

I'm intrigued from the commit that you say it's working for serve but not for build, Does that assume that so long as we've already got some pre-compiled assets from old builds, then we're good?

So the last task is to ensure that assets unchanged here need to get through the build step?

kr8n3r commented 2 months ago

the newly created govuk_frontend.js does not get generated in the build step, not sure if it's the limitation of the asset pipeline or lib setup or something else. As said, ideally we don't want downstream users to change anything

kr8n3r commented 2 months ago

exploration using middleman external pipeline with gulp https://github.com/alphagov/tech-docs-gem/commit/26f13f0ea6442c2ea6775b5c5762fecc03b6ecbf

kr8n3r commented 2 months ago

We've explored this a bit further with @huwd , summary below

Problems

Compile Sass

Sass files are currently compiled with SassC, which with govuk-frontend v5 that includes source maps, fails to compile styles correctly - only including source maps. This could potentially be resolved by replacing SassC with a Dart Sass conduit for Ruby allowing us to keep the current asset pipeline as-is.

Compile Javascript

govuk-frontend v5 dropped support for legacy browsers, so the preference when using it is to included as a type=module That requires a new entry point in the core layout and a way to compile it / include it in addition to the current application.js as we do not want to make everyone change the way they write their own javascript in downstream projects.

Current flow is that downstream project has an application.js which requires any of the own local files, as well as govuk_tech_docs.js which comes from this gem. In turn, govuk_tech_docs.js requires govuk-frontend, jquery, etc. So when, downstream run bundle exec middleman build everything gets compiled into application.js.

For govuk-frontend v5 we want to compile two files:

To try and achieve that in a way where a downstream project doesn't have to make any additions we explored

a) creating a govuk_frontend.js entry point in the lib: https://github.com/alphagov/tech-docs-gem/commit/39502c0c628456cf0780174987a2723fce72d0a9 b) using Gulp with Middleman external pipeline: https://github.com/alphagov/tech-docs-gem/commit/26f13f0ea6442c2ea6775b5c5762fecc03b6ecbf

WIth a) the problem found was that the generated govuk-frontend.js was served with middleman serve but not present in the build folder when middleman build was run.

With b) the problem is that any script to run say gulp with the gulpfile, would need to be present in the downstream projects package.json. We could not find a way to execute the script that lies in this gem. There is a hacky way to extract the path of the gulpfile from the the gem, but it doesn't feel right.

Using Middleman external pipeline feels like the way forward, but we'd need to find a way for a downstream project to trigger gem build steps on serve and build without needing to any additional installation steps.

Should above not be achievable, I believe the only other way to achieve separate entry points is:

Then their build should create all the necessary files. It's a similar flow to how application.js is constructed.

huwd commented 2 months ago

Bit I want to really exhaust is "are we really sure we can't do this with sprockets".

Things We've discovered:

Bits left for me to work out:

I'd really like to exhaust this line of thought as it's the least disruptive to the current delivery model of the tech-doc-gem, and keeps batteries included as much as possible without asking tech-doc folk to figure out how to run an external asset pipeline (which I think raises the congitive load for folks to use this)

huwd commented 2 months ago

@kr8n3r I think i've found something in sprockets that might help Adding this line: https://github.com/alphagov/tech-docs-gem/commit/38fa85d1d3be541a10b3e4785694c6bbbdc17954

Creates a new file under assets/govuk/govuk-frontend.min.js which I think I should be able to reference as a separate entry point, no?

I testing that it does actually bring something across by creating a bloop.js that does console.log inside the node modules folder, adding that and I can now see it in by build directory downstream... so perhaps there's a lead there?

kevindew commented 2 months ago

Rather than having to tie everything together in gem build and serve steps, I'd suggest going the rails route and having a bin/dev executable which can manage multiple processes. Then you can have dart sass process there and even a JS build too if needs be.

You can put the executable in an exe directory to have it bundled as part of the gem and could maybe explore setting the executable up as a binstub that installers of the gem use.

kr8n3r commented 2 months ago

@huwd this produces an error during the build (CJS) as i don't think this pipeline handles es6 (esm). As this is a minified ready to go file, I'll look into copying it directly like we do with other assets

error is because minify_js only seemn works with ES5. for es6 ruby-terser looks to be a better option. if only we can use that with middleman-sprockets

so if we can switch minifiers, then we can provide a file where downstream can include it with a one-line in a new file see https://github.com/alphagov/tech-docs-gem/commit/f5e863c009588625ce41fe3ac4449af69c8cfc63

Screenshot 2024-09-17 at 09 58 01 Screenshot 2024-09-17 at 09 48 07 Screenshot 2024-09-17 at 09 46 57
huwd commented 2 months ago

@kr8n3r what do we think about @kevindew's line above?

I hadn't considered using /bin/dev in that way... could be a way to get us shifting to the "external pipeline" model, dropping a bunch of old dependencies and bringing in modern / quick ones...

... but if I understand @kevindew's comment right still having this as something that ships with the gem, and so keeps the feature set here as "batteries included", rather than pushing the burden down for every docsite to manage itself.

kr8n3r commented 2 months ago

not my area of expertise so cannot comment. working draft of the current pipeline with v5 https://github.com/alphagov/tech-docs-gem/pull/363

kr8n3r commented 1 month ago

https://github.com/alphagov/tech-docs-gem/pull/375