-
Notifications
You must be signed in to change notification settings - Fork 338
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 components relying on global builds #588
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ | |
"build:dist": "node bin/check-nvmrc.js && gulp build:dist --destination 'dist' && npm run test:build:dist", | ||
"test": "standard && gulp test && npm run test:app && npm run test:components && npm run test:generate:readme", | ||
"test:app": "jest app/__tests__/app.test.js --forceExit # express server fails to end process", | ||
"test:components": "jest src/", | ||
"test:components": "jest src/ && jest tasks/gulp/__tests__/check-individual-components-compile.test.js", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this run all tests under There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some of the task tests run 'after' tasks. So they're dependant on a task running. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
"test:generate:readme": "jest tasks/gulp/__tests__/check-generate-readme.test.js", | ||
"test:build:packages": "jest tasks/gulp/__tests__/after-build-packages.test.js", | ||
"test:build:dist": "jest tasks/gulp/__tests__/after-build-dist.test.js" | ||
|
@@ -57,6 +57,7 @@ | |
"js-yaml": "^3.10.0", | ||
"lerna": "^2.3.1", | ||
"merge-stream": "^1.0.1", | ||
"node-sass": "^4.7.2", | ||
"nodemon": "^1.12.1", | ||
"oldie": "^1.3.0", | ||
"postcss-normalize": "^3.0.0", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
@import "../globals/common"; | ||
|
||
@include govuk-exports("footer") { | ||
|
||
$govuk-footer-background: $govuk-grey-3; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* eslint-env jest */ | ||
|
||
const path = require('path') | ||
|
||
const sass = require('node-sass') | ||
|
||
const lib = require('../../../lib/file-helper') | ||
const configPaths = require('../../../config/paths.json') | ||
|
||
const sassRender = (options) => { | ||
return new Promise((resolve, reject) => { | ||
sass.render(options, (error, result) => { | ||
if (error) { | ||
return reject(error) | ||
} | ||
return resolve(result) | ||
}) | ||
}) | ||
} | ||
|
||
describe('Individual components', () => { | ||
it('should compile individual scss files without throwing exceptions', done => { | ||
const componentNames = lib.SrcFilteredComponentList.slice() | ||
|
||
const getSassRenders = () => { | ||
return componentNames.map(name => { | ||
const filePath = path.join(configPaths.src, name, `_${name}.scss`) | ||
return sassRender({ file: filePath }) | ||
}) | ||
} | ||
|
||
Promise | ||
.all(getSassRenders()) | ||
.then(() => { done() }) | ||
.catch(error => { throw error }) | ||
}) | ||
}) |
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.
Given footer hasn't been released yet, is this something we need to document? As the thing we're fixing was never actually released?
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.
Agreed, that's why this change is under 'Internal' not 'Fix'