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.

Sign up for GitHub

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

Jump to bottom

Run browser tests on minified code #3050

Merged
merged 4 commits into from
Dec 23, 2022
Merged

Run browser tests on minified code #3050

merged 4 commits into from
Dec 23, 2022

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Nov 29, 2022 •
edited
Loading

Quick proof of concept to:

  1. Run the JS minifier on all.mjs (for ./dist and ./public)
  2. Run the CSS minifier on all Sass files
  3. Use minified files in the review app
  4. Use .min.* extensions in webpack example

Discussed in:

Why?

Ensures our browser tests (via the review app) run on minified code as minifiers can cause their own issues:

Also gives us the confidence to swap to a modern ES6+ minifier:

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3050 November 29, 2022 15:32 Inactive
@colinrotherham colinrotherham mentioned this pull request Nov 29, 2022
Merged
@colinrotherham colinrotherham marked this pull request as draft November 29, 2022 15:33
@colinrotherham
Copy link
Contributor Author

Question: Do we need to keep the (unminified) all.js bundle alongside all.min.js?

@colinrotherham colinrotherham changed the title In discussion: Minify bundled code by default For discussion: Minify bundled code by default Nov 29, 2022
@colinrotherham colinrotherham linked an issue Nov 29, 2022 that may be closed by this pull request
Split project into separate app, config, lib and tasks packages #2886
Closed
7 tasks
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3050 December 8, 2022 08:59 Inactive
@colinrotherham colinrotherham changed the base branch from main to rollup-standalone December 9, 2022 16:53
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3050 December 9, 2022 16:53 Inactive
@colinrotherham colinrotherham force-pushed the rollup-standalone branch 2 times, most recently from dc5fe68 to 88d6f4f Compare December 12, 2022 16:26
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3050 December 12, 2022 16:31 Inactive
@colinrotherham colinrotherham force-pushed the rollup-standalone branch 4 times, most recently from 96d4c8a to e0ed994 Compare December 12, 2022 19:07
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3050 December 12, 2022 19:40 Inactive
@colinrotherham colinrotherham changed the title For discussion: Minify bundled code by default Minify review app code by default Dec 13, 2022
@colinrotherham colinrotherham marked this pull request as ready for review December 13, 2022 09:35
@colinrotherham colinrotherham requested a review from a team December 13, 2022 09:37
@colinrotherham colinrotherham force-pushed the rollup-standalone branch 2 times, most recently from 5eb4cbc to 94bf08f Compare December 20, 2022 11:39
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3050 December 20, 2022 11:43 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3050 December 20, 2022 11:49 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3050 December 20, 2022 11:57 Inactive
@colinrotherham
Copy link
Contributor Author

@36degrees This one is ready for review again

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3050 December 20, 2022 13:54 Inactive
@colinrotherham
Copy link
Contributor Author

Rebased with:

Base automatically changed from rollup-standalone to main December 20, 2022 14:34
@colinrotherham colinrotherham changed the title Minify review app code by default Run browser tests on minified code Dec 23, 2022
@colinrotherham
Always minify CSS by default
4033971
@colinrotherham
Review app all.js script sources now all.min.js
8df33f4 
We now compile to `all.min.js` to ensure our browser tests can run on minified code
@colinrotherham
Review app html5shiv.js script sources now html5shiv.min.js
5bd2366
@colinrotherham
Prefer .min.* extensions in webpack example
356a30c 
For consistency, shows that file content is minified
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3050 December 23, 2022 11:58 Inactive
@36degrees 36degrees mentioned this pull request Dec 23, 2022
Closed
36degrees
36degrees approved these changes Dec 23, 2022
View reviewed changes
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

LGTM.

This uncovers the fact that our minified JS (which we include in dist/ and include in the zip attached to each release) errors in IE8. I don't think that should block this (as it appears to be a pre-existing issue) – have raised #3136 to track separately.

@colinrotherham colinrotherham merged commit a6f0493 into main Dec 23, 2022
@colinrotherham colinrotherham deleted the minify-all-by-default branch December 23, 2022 13:22
@colinrotherham colinrotherham mentioned this pull request Jan 18, 2023
Merged
@colinrotherham colinrotherham mentioned this pull request Apr 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romaricpascal romaricpascal romaricpascal left review comments

@36degrees 36degrees 36degrees approved these changes

Assignees
No one assigned
Labels
javascript sass / css
Projects
GOV.UK Design System cycle board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Split project into separate app, config, lib and tasks packages
4 participants
@colinrotherham @36degrees @romaricpascal @govuk-design-system-ci