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

Add required imports to form-group #1226

Closed
wants to merge 1 commit into from
Closed

Conversation

penx
Copy link

@penx penx commented Feb 28, 2019

When doing:

@import "~govuk-frontend/objects/form-group";

I get:

ERROR in ./src/objects/form-group/_form-group.module.scss (./node_modules/css-loader??ref--6-1!./node_modules/sass-loader/lib/loader.js!./src/objects/form-group/_form-group.module.scss)
Module build failed (from ./node_modules/sass-loader/lib/loader.js):

@include govuk-exports("govuk/objects/form-group") {
        ^
      No mixin named govuk-exports
      in /node_modules/govuk-frontend/objects/_form-group.scss (line 1, column 10)

Adding

@import "~govuk-frontend/settings/all";
@import "~govuk-frontend/helpers/all";

Makes it compile ok, and adding

@import "~govuk-frontend/tools/all";

Seems to be needed to get the correct CSS output.

In my opinion this would be better addressed in govuk-frontend direct.

When doing:

```scss
@import "~govuk-frontend/objects/form-group";
```

I get:

```
ERROR in ./src/objects/form-group/_form-group.module.scss (./node_modules/css-loader??ref--6-1!./node_modules/sass-loader/lib/loader.js!./src/objects/form-group/_form-group.module.scss)
Module build failed (from ./node_modules/sass-loader/lib/loader.js):

@include govuk-exports("govuk/objects/form-group") {
        ^
      No mixin named govuk-exports
      in /node_modules/govuk-frontend/objects/_form-group.scss (line 1, column 10)
```

Adding

```scss
@import "~govuk-frontend/settings/all";
@import "~govuk-frontend/helpers/all";
```

Makes it compile ok, and adding

```scss
@import "~govuk-frontend/tools/all";
```

Seems to be needed to get the correct CSS output.
@penx
Copy link
Author

penx commented Feb 28, 2019

@penx
Copy link
Author

penx commented Feb 28, 2019

This also needs to be reviewed on other Scss files in objects, e.g. width-container.

@hannalaakso hannalaakso added the awaiting triage Needs triaging by team label Mar 1, 2019
@NickColley
Copy link
Contributor

NickColley commented Mar 1, 2019

Hey @penx !

As far as I can tell, it does look like these files should be declaring their dependencies like you've described.

I've had a quick look and this looks to be the same case with most of the files, I'll ping the team to see if this was intentional.

We could try to have a high level smoke test that imports all files on their own and see if they fail?

@36degrees
Copy link
Contributor

@penx are you happy that this would be addressed by #1235?

@penx
Copy link
Author

penx commented Mar 5, 2019

Yep, great work thanks!

@36degrees
Copy link
Contributor

Closing in favour of #1235 – thanks again, @penx 👍

@36degrees 36degrees closed this Mar 5, 2019
@kellylee-gds kellylee-gds removed the awaiting triage Needs triaging by team label Mar 20, 2019
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.

5 participants