-
Notifications
You must be signed in to change notification settings - Fork 334
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
Namespace exports with 'govuk' #732
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Thought – do we want to standardise on how we construct these strings to avoid conflicts across layers? e.g. |
@36degrees can do, i see no issue with it. will amend |
@36degrees updated the exports with layer names |
@@ -2,7 +2,7 @@ | |||
@import "../../tools/all"; | |||
@import "../../helpers/all"; | |||
|
|||
@include govuk-exports("back-link") { | |||
@include govuk-exports("govuk/component/back-link") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'component' -> 'components'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue for the use of singular
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit weird having component, but helpers, objects, utilities. But probably not going back and changing now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not part of the public api, so not too much trouble
We have found that an application using the same export as us, it will conflict resulting in a broken build.
For example, when releasing the Header component, it conflicted with the application level export.
This fixes: #724
Trello ticket: https://trello.com/c/UaX5dWyO/1025-namespace-all-scss-exports-with-govuk