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

Build tasks: Move review app tasks into app/tasks #3384

Merged
merged 9 commits into from
Mar 30, 2023
Merged
5 changes: 4 additions & 1 deletion .github/workflows/screenshots.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ jobs:
uses: ./.github/workflows/actions/build

- name: Start review application
run: npm run serve &

# Review app is already built (or restored from cache) so
# start with `--ignore-scripts` to prevent "prestart" build
run: npm start --ignore-scripts --workspace app &

- name: Send screenshots to Percy
run: npx percy exec --parallel -- npm run test:screenshots
Expand Down
2 changes: 1 addition & 1 deletion Procfile
Original file line number Diff line number Diff line change
@@ -1 +1 @@
web: npm run heroku
web: npm start --workspace app
16 changes: 16 additions & 0 deletions app/gulpfile.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import gulp from 'gulp'

import * as build from './tasks/build/index.mjs'
import { scripts, styles } from './tasks/index.mjs'

/**
* Build target tasks
*/
gulp.task('build', build.dist)
gulp.task('dev', build.dev)

/**
* Utility tasks
*/
gulp.task('scripts', scripts)
gulp.task('styles', styles)
2 changes: 1 addition & 1 deletion app/nodemon.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"watch": ["./src", "../src/govuk"],
"watch": ["./src", "../src/govuk/**/*.{json,yaml}"],
"ignore": ["**/*.test.*"],
"ext": "mjs,json,yaml",
"quiet": true
Expand Down
8 changes: 7 additions & 1 deletion app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
},
"license": "MIT",
"scripts": {
"build": "gulp build",
"dev": "gulp dev",
"serve": "nodemon",
"prestart": "npm run build",
Copy link
Member

Choose a reason for hiding this comment

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

nitpick As we're looking at reducing the complexity of our build, I think I'd be keen to avoid that prestart step that magically links the build to all our start calls. It also allows to keep the build and the start of the app separate without having to know that you need to reach for --ignore-scripts (which is a bit arcane).

I think it'll only be used on heroku so could we maybe make things explicit in the Procfile with something like (cd app && run build && npm start) (or with two --workspace app on each command).

Copy link
Contributor Author

@colinrotherham colinrotherham Mar 30, 2023

Choose a reason for hiding this comment

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

Would you mind if we kept npm start?

Keeps things simple

Although perhaps in future should align start/dev?

Server start

npm start
npm start --workspace app
npm start --workspace docs/examples/webpack

Development

npm run dev
npm run dev --workspace app
npm run dev --workspace docs/examples/webpack

But for now we have a necessary mismatch with npm start at project-level running this:

npm run dev --workspace app

"start": "node src/start.mjs"
},
"dependencies": {
Expand All @@ -21,6 +24,8 @@
"govuk_frontend_toolkit": "^9.0.1",
"govuk_template_jinja": "^0.26.0",
"govuk-elements-sass": "3.1.3",
"gulp": "^4.0.2",
"gulp-cli": "^2.3.0",
"highlight.js": "^11.7.0",
"html5shiv": "^3.7.3",
"iframe-resizer": "3.5.15",
Expand All @@ -32,7 +37,8 @@
"nodemon": "^2.0.22",
"nunjucks": "^3.2.3",
"outdent": "^0.8.0",
"shuffle-seed": "^1.1.6"
"shuffle-seed": "^1.1.6",
"slash": "^5.0.0"
},
"devDependencies": {
"cheerio": "^1.0.0-rc.12",
Expand Down
14 changes: 14 additions & 0 deletions app/tasks/assets.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { join } from 'path'

import { paths } from '../../config/index.js'
import { files } from '../../tasks/index.mjs'

/**
* Copy GOV.UK Frontend static assets
*/
export async function assets () {
await files.copy('**/*', {
srcPath: join(paths.src, 'govuk/assets'),
destPath: join(paths.app, 'dist/assets')
})
}
17 changes: 17 additions & 0 deletions app/tasks/build/dev.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import gulp from 'gulp'

import { paths } from '../../../config/index.js'
import { npm } from '../../../tasks/index.mjs'
import { watch } from '../index.mjs'

import dist from './dist.mjs'

/**
* Dev task
* Runs a sequence of tasks on start
*/
export default gulp.series(
dist,
watch,
npm.script('serve', paths.app)
)
16 changes: 16 additions & 0 deletions app/tasks/build/dist.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import gulp from 'gulp'

import { assets, clean, scripts, styles } from '../index.mjs'

/**
* Build review app task
* Prepare dist folder for review app
*/
export default gulp.series(
clean,
gulp.parallel(
assets,
scripts,
styles
)
)
5 changes: 5 additions & 0 deletions app/tasks/build/index.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/**
* Build target tasks
*/
export { default as dev } from './dev.mjs'
export { default as dist } from './dist.mjs'
13 changes: 13 additions & 0 deletions app/tasks/clean.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { join } from 'path'

import { paths } from '../../config/index.js'
import { files } from '../../tasks/index.mjs'

/**
* Clean task
*/
export async function clean () {
await files.clean('**/*', {
destPath: join(paths.app, 'dist')
})
}
8 changes: 8 additions & 0 deletions app/tasks/index.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* Build tasks
*/
export { assets } from './assets.mjs'
export { clean } from './clean.mjs'
export { compile as scripts } from './scripts.mjs'
export { compile as styles } from './styles.mjs'
export { watch } from './watch.mjs'
26 changes: 26 additions & 0 deletions app/tasks/scripts.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { join } from 'path'

import gulp from 'gulp'

import { paths } from '../../config/index.js'
import { npm, scripts, task } from '../../tasks/index.mjs'

/**
* JavaScripts task (for watch)
* Compilation, documentation
*/
export const compile = gulp.series(
task.name('compile:js', () =>
scripts.compile('all.mjs', {
srcPath: join(paths.src, 'govuk'),
destPath: join(paths.app, 'dist/javascripts'),

filePath (file) {
return join(file.dir, `${file.name}.min.js`)
}
})
),

// Build JSDoc for /docs/javascript
npm.script('build:jsdoc')
)
26 changes: 26 additions & 0 deletions app/tasks/styles.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { join } from 'path'

import gulp from 'gulp'

import { paths } from '../../config/index.js'
import { npm, styles, task } from '../../tasks/index.mjs'

/**
* Stylesheets task (for watch)
* Compilation, documentation
*/
export const compile = gulp.series(
task.name('compile:scss', () =>
styles.compile('**/[!_]*.scss', {
srcPath: join(paths.app, 'src/stylesheets'),
destPath: join(paths.app, 'dist/stylesheets'),

filePath (file) {
return join(file.dir, `${file.name}.min.css`)
}
})
),

// Build SassDoc for /docs/sass
npm.script('build:sassdoc')
)
18 changes: 9 additions & 9 deletions tasks/gulp/watch.mjs → app/tasks/watch.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import gulp from 'gulp'
import slash from 'slash'

import { paths } from '../../config/index.js'
import { npm } from '../index.mjs'
import { npm } from '../../tasks/index.mjs'

import { scripts, styles } from './index.mjs'

/**
* Watch task
Expand All @@ -15,23 +17,21 @@ import { npm } from '../index.mjs'
export function watch () {
return Promise.all([
gulp.watch([
'sassdoc.config.yaml',
`${slash(paths.root)}/sassdoc.config.yaml`,
`${slash(paths.app)}/src/**/*.scss`,
`${slash(paths.src)}/govuk/**/*.scss`,
`!${slash(paths.src)}/govuk/vendor/*`
], gulp.parallel(
npm.run('lint:scss'),
'styles'
npm.script('lint:scss'),
styles
)),

gulp.watch([
'jsdoc.config.js',
`${slash(paths.root)}/jsdoc.config.js`,
`${slash(paths.src)}/govuk/**/*.mjs`
], gulp.parallel(
npm.run('lint:js'),
'scripts'
npm.script('lint:js'),
scripts
))
])
}

watch.displayName = 'watch'
4 changes: 2 additions & 2 deletions app/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"extends": "../tsconfig.base.json",
"include": ["./src/**/*.mjs", "./src/**/*.json"],
"exclude": ["./src/**/*.test.*"],
"include": ["**/*.mjs", "**/*.json"],
"exclude": ["**/*.test.*", "./dist/**"],
"compilerOptions": {
"resolveJsonModule": true
}
Expand Down
35 changes: 16 additions & 19 deletions docs/contributing/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,38 +8,31 @@ To run the application without any tasks being triggered, see [Express app only]

npm scripts are defined in `package.json`. These trigger a number of Gulp tasks.

**`npm start` will trigger `gulp dev` that will:**
**`npm start` will trigger `npm run dev --workspace app` that will:**

- clean the `./app/dist` folder
- copy fonts and images
- compile JavaScript and Sass, including documentation
- compile again when `.scss` and `.mjs` files change
- runs `npm run serve`
- runs `npm run serve --workspace app`

**`npm test` will do the following:**

- run Nunjucks macros tests
- run JavaScript tests on the review application
- run accessibility and HTML validation tests

**`npm run serve` will do the following:**
**`npm run serve --workspace app` will do the following:**

- start up Express, restarting when `.js` files change
- start up Express, restarting when `.mjs`, `.json` or `.yaml` files change

**`npm run heroku` runs on Heroku build/PR and it will:**

- run `npm run build:app`
- start up Express

**`npm run build:app` will do the following:**
**`npm run build:app` will trigger `npm run build --workspace app` that will:**

- clean the `./app/dist` folder
- output files into `./app/dist`
- copy fonts and images
- run sub tasks from `gulp styles` without StyleLint code quality checks
- run sub tasks from `gulp scripts` without ESLint code quality checks
- compile Sass documentation into `./app/dist/docs/sassdoc`
- compile JavaScript documentation into `./app/dist/docs/jsdoc`
- compile Sass to CSS, including documentation
- compile JavaScript ESM to CommonJS, including documentation

**`npm run build:package` will do the following:**

Expand All @@ -50,7 +43,7 @@ npm scripts are defined in `package.json`. These trigger a number of Gulp tasks.
- copy GOV.UK Prototype Kit config files
- copy JavaScript ESM source files
- compile JavaScript ESM to CommonJS
- runs `npm run test:build:package` (which will test the output is correct)
- runs `npm run postbuild:package` (which will test the output is correct)

**`npm run build:dist` will do the following:**

Expand All @@ -59,27 +52,31 @@ npm scripts are defined in `package.json`. These trigger a number of Gulp tasks.
- copy fonts and images
- compile JavaScript and Sass
- append version number from `package/package.json` to compiled JavaScript and CSS files
- runs `npm run test:build:dist` (which will test the output is correct)
- runs `npm run postbuild:dist` (which will test the output is correct)

## Gulp tasks

Gulp tasks are defined in `gulpfile.mjs` and .`/tasks/` folder.
Project Gulp tasks are defined in [`gulpfile.mjs`](../../gulpfile.mjs) and the [`tasks/`](../../tasks) folder.

**`gulp --tasks`**

This task will:

- list out all available tasks

**`gulp styles`**
Review app Gulp tasks are defined in [`app/gulpfile.mjs`](../../app/gulpfile.mjs) and the [`app/tasks/`](../../app/tasks) folder.

Gulp tasks from npm workspaces (such as the review app) can be run as shown:

**`npx --workspace app -- gulp styles`**

This task will:

- check Sass code quality via Stylelint (`npm run lint:scss`)
- compile Sass to CSS into `./app/dist/stylesheets`
- compile Sass documentation into `./app/dist/docs/sassdoc`

**`gulp scripts`**
**`npx --workspace app -- gulp scripts`**

This task will:

Expand Down
Loading