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

Fix govuk-frontend deprecation on component_support #1934

Closed
wants to merge 1 commit into from

Conversation

kevindew
Copy link
Member

@kevindew kevindew commented Feb 16, 2021

govuk-frontend now expects govuk/base to be imported before any other
files.

This fixes deprecation warnings such as:

I, [2021-02-16T11:17:55.276774 #25445]  INFO -- : Writing /Users/kevindew/govuk/collections/public/assets/collections/component_guide/visual-regression-7a6fbecb638d5f96b24d97212b0044b0b3d9e195c0ea5543612dba4347f5ec2e.js.gz
WARNING: Importing items from the core layer without first importing `base` is deprecated, and will no longer work as of GOV.UK Frontend v4.0.
         on line 2:3 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/node_modules/govuk-frontend/govuk/core/_links.scss
         from line 1:9 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/node_modules/govuk-frontend/govuk/core/_all.scss
         from line 4:9 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/app/assets/stylesheets/govuk_publishing_components/component_support.scss
         from line 5:9 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/app/assets/stylesheets/govuk_publishing_components/components/print/_accordion.scss
         from line 6:9 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/app/assets/stylesheets/govuk_publishing_components/_all_components_print.scss
         from line 1:9 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/app/assets/stylesheets/component_guide/print.scss

This actual import doesn't output any CSS it just imports files that, as
of GOV.UK Frontend 3.9, are imported automatically 1 and this will no
longer happen in GOV.UK Frontend 4.0.

@bevanloon bevanloon temporarily deployed to govuk-publis-fix-govuk--ai0ju8 February 16, 2021 11:29 Inactive
govuk-frontend now expects govuk/base to be imported before any other
files.

This fixes deprecation warnings such as:

```
I, [2021-02-16T11:17:55.276774 #25445]  INFO -- : Writing /Users/kevindew/govuk/collections/public/assets/collections/component_guide/visual-regression-7a6fbecb638d5f96b24d97212b0044b0b3d9e195c0ea5543612dba4347f5ec2e.js.gz
WARNING: Importing items from the core layer without first importing `base` is deprecated, and will no longer work as of GOV.UK Frontend v4.0.
         on line 2:3 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/node_modules/govuk-frontend/govuk/core/_links.scss
         from line 1:9 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/node_modules/govuk-frontend/govuk/core/_all.scss
         from line 4:9 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/app/assets/stylesheets/govuk_publishing_components/component_support.scss
         from line 5:9 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/app/assets/stylesheets/govuk_publishing_components/components/print/_accordion.scss
         from line 6:9 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/app/assets/stylesheets/govuk_publishing_components/_all_components_print.scss
         from line 1:9 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/app/assets/stylesheets/component_guide/print.scss
```

This actual import doesn't output any CSS it just imports files that, as
of GOV.UK Frontend 3.9, are imported automatically [1] and this will no
longer happen in GOV.UK Frontend 4.0.

[1]: https://github.com/alphagov/govuk-frontend/blob/8748418071b7e753a2085b409d10e1c2fb40990e/src/govuk/core/_links.scss#L1-L5
@kevindew kevindew force-pushed the fix-govuk-frontend-deprecation branch from 87bbfaf to 620f441 Compare February 16, 2021 11:30
@bevanloon bevanloon temporarily deployed to govuk-publis-fix-govuk--ai0ju8 February 16, 2021 11:31 Inactive
Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

I looked into this wraning a few days ago too and I think, in the current setup, our component_support.scss must import govuk_frontend_support.scss by default instead of doing it separately/optionally at the app level, otherwise we'll include govuk/base twice.

@kevindew
Copy link
Member Author

I looked into this error a few days ago too and I think, in the current setup, our component_support.scss must import govuk_frontend_support.scss by default instead of doing it separately/optionally at the app level, otherwise we'll include govuk/base twice.

Oh nice one.

So I notice that this isn't the case for the print stylesheet in collections, which is why we're seeing this warning - and I think it might be the case across all print stylesheets?

What I'm a bit unclear on is why we need both component_support and govuk_frontend_support - and why component_support imports in the base bits of govuk-frontend. I wonder if their purposes are a bit unclear which might be how this problem started forming?

@andysellick
Copy link
Contributor

What I'm a bit unclear on is why we need both component_support and govuk_frontend_support - and why component_support imports in the base bits of govuk-frontend. I wonder if their purposes are a bit unclear which might be how this problem started forming?

I've been meaning to document this properly for a while (although I think there are comments at the top of each file explaining what they do). In short, all the apps use the gem but static only needs the mixins and variables, which add no CSS weight. If we included everything everywhere we'd duplicate govuk-frontend's CSS across the whole of GOV.UK.

@kevindew
Copy link
Member Author

What I'm a bit unclear on is why we need both component_support and govuk_frontend_support - and why component_support imports in the base bits of govuk-frontend. I wonder if their purposes are a bit unclear which might be how this problem started forming?

I've been meaning to document this properly for a while (although I think there are comments at the top of each file explaining what they do). In short, all the apps use the gem but static only needs the mixins and variables, which add no CSS weight. If we included everything everywhere we'd duplicate govuk-frontend's CSS across the whole of GOV.UK.

Thank you 🙂 I still find myself a bit confused as to whether govuk_frontend_support is a mandatory import or not? I'm a bit unclear if print stylesheets should have it in, and if they should whether something should be breaking somewhere when they're not.

@andysellick
Copy link
Contributor

Thank you 🙂 I still find myself a bit confused as to whether govuk_frontend_support is a mandatory import or not? I'm a bit unclear if print stylesheets should have it in, and if they should whether something should be breaking somewhere when they're not.

I think it's mostly mandatory if not entirely, but that might be worth further investigation/documenting at some point.

@kevindew
Copy link
Member Author

Thank you 🙂 I still find myself a bit confused as to whether govuk_frontend_support is a mandatory import or not? I'm a bit unclear if print stylesheets should have it in, and if they should whether something should be breaking somewhere when they're not.

I think it's mostly mandatory if not entirely, but that might be worth further investigation/documenting at some point.

Cool thanks. So it sounds like it's best to edit the stylesheets that are emitting this warning to import "govuk_frontend_support" rather than merging this PR?

I thought initially that it could have been all print stylesheets, as my quick sample of a couple of apps had the same issues, however from a quick scan of my ~/govuk directory (which may be out of date and has various legacy things) it looks like there's only a few that miss it - with the component-guide/print.scss being the one that likely causes the majority of warnings.

➜  ~ find govuk -type f -name print.scss -exec grep -L govuk_frontend_support {} \;
govuk/govuk-user-intent-survey-explorer/app/assets/stylesheets/print.scss
govuk/govuk-coronavirus-vulnerable-people-form/app/assets/stylesheets/print.scss
govuk/release/app/assets/stylesheets/print.scss
govuk/content-publisher/app/assets/stylesheets/print.scss
govuk/collections-publisher/app/assets/stylesheets/print.scss
govuk/govuk_publishing_components/app/assets/stylesheets/component_guide/print.scss
govuk/govuk_publishing_components/spec/dummy/app/assets/stylesheets/print.scss
govuk/search-admin/app/assets/stylesheets/print.scss
govuk/govspeak-preview/vendor/bundle/ruby/2.6.0/gems/govuk_publishing_components-21.63.2/app/assets/stylesheets/component_guide/print.scss

@kevindew
Copy link
Member Author

kevindew commented Mar 8, 2021

Closed in favour of #1961

@kevindew kevindew closed this Mar 8, 2021
@alex-ju alex-ju deleted the fix-govuk-frontend-deprecation branch March 8, 2021 11:48
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