Skip to content
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

Publishing components upgrade #1647

Merged
merged 3 commits into from
Jul 22, 2024
Merged

Publishing components upgrade #1647

merged 3 commits into from
Jul 22, 2024

Conversation

callumknights
Copy link
Contributor

@callumknights callumknights commented Jul 12, 2024

Trello: https://trello.com/c/E0hw9QCZ/1241-upgrade-govukpublishingcomponents-for-transition

This application is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

@callumknights callumknights force-pushed the publishing-components-upgrade branch 8 times, most recently from ee82769 to 68a4616 Compare July 16, 2024 08:56
Copy link
Member

@brucebolt brucebolt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good to me, except for two really minor things I've left comments for.

app/assets/javascripts/es6-components.js Outdated Show resolved Hide resolved
Gemfile.lock Show resolved Hide resolved
@callumknights callumknights force-pushed the publishing-components-upgrade branch from 68a4616 to b78a37d Compare July 17, 2024 14:11
@callumknights
Copy link
Contributor Author

I've also had a separate conversation with @MartinJJones @brucebolt, I'm gonna merge this with the commit upgrading Publishing-components left in, and close the dependabot PR.

Copy link
Member

@brucebolt brucebolt left a 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 👍 . Just remember to remove the DO-NOT-MERGE label from the PR title before merging (otherwise it ends up in the commit history).

@mike21573 mike21573 changed the title [DO-NOT-MERGE] Publishing components upgrade Publishing components upgrade Jul 22, 2024
This replaces Uglifier, as Terser supports ES6
This change is part of the process for upgrading apps to a new version of
govuk_publishing_components that uses GOV.UK frontend V5.1

This is one of the steps detailed in the [Upgrading to GOV.UK Frontend v5 document](https://docs.google.com/document/d/1uwip7pzQwM7t5ghn9_8KrsWXLePSRUltFPZ5fYw6Ab8/edit).

The purpose of this is to prevent browsers that don't support ES6 attempting to load ES6 Javascript from our components.

We know browser that don't support ES6 as grade X browsers, and these browsers also don't support
the `type = "module"` attribute in script tags. So we use this to prevent such code being loaded by ES6 Browsers.

The 'all_components' require as been removed to avoid grade X browsers calling ES6 javascript
@mike21573 mike21573 force-pushed the publishing-components-upgrade branch from b78a37d to c154b45 Compare July 22, 2024 10:00
@mike21573 mike21573 merged commit 5987ee2 into main Jul 22, 2024
10 checks passed
@mike21573 mike21573 deleted the publishing-components-upgrade branch July 22, 2024 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants