Skip to content

Commit

Permalink
Prefer NODE_ENV to determine test or production builds
Browse files Browse the repository at this point in the history
Fixes an issue where PostCSS automatically defaults `{ env: 'development' }`

We previously run `postcss.config.unit.test.mjs` in a JSDOM environment which removed `NODE_ENV`
  • Loading branch information
colinrotherham committed Sep 12, 2023
1 parent b91c949 commit 49c229e
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 78 deletions.
19 changes: 19 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
"predev": "npm run build",
"dev": "concurrently \"npm run dev --workspace @govuk-frontend/review\" \"npm run dev --workspace govuk-frontend\" --kill-others --names \"app,pkg\" --prefix-colors \"red.dim,blue.dim\"",
"build": "npm run build --workspace govuk-frontend --workspace @govuk-frontend/review",
"build:app": "npm run build --workspace @govuk-frontend/review",
"build:package": "npm run build:package --workspace govuk-frontend",
"build:release": "npm run build:release --workspace govuk-frontend",
"build:app": "cross-env-shell NODE_ENV=production npm run build --workspace @govuk-frontend/review",
"build:package": "cross-env-shell NODE_ENV=production npm run build:package --workspace govuk-frontend",
"build:release": "cross-env-shell NODE_ENV=production npm run build:release --workspace govuk-frontend",
"build:types": "npm run build:types --workspaces --if-present",
"postbuild:package": "jest --color --selectProjects \"Build tasks\" --testMatch \"**/build/package.unit.test.mjs\"",
"postbuild:release": "jest --color --selectProjects \"Build tasks\" --testMatch \"**/build/release.unit.test.mjs\"",
Expand Down Expand Up @@ -55,6 +55,7 @@
"@typescript-eslint/eslint-plugin": "^6.6.0",
"@typescript-eslint/parser": "^6.6.0",
"concurrently": "^8.2.1",
"cross-env": "^7.0.3",
"editorconfig-checker": "^5.1.1",
"eslint": "^8.49.0",
"eslint-config-prettier": "^9.0.0",
Expand Down
7 changes: 4 additions & 3 deletions packages/govuk-frontend/postcss.config.mjs
Original file line number Diff line number Diff line change
@@ -1,24 +1,25 @@
import { pkg } from '@govuk-frontend/config'
import { isDev } from '@govuk-frontend/tasks/helpers/task-arguments.mjs'
import autoprefixer from 'autoprefixer'
import cssnano from 'cssnano'
import cssnanoPresetDefault from 'cssnano-preset-default'
import postcss from 'postcss'
import scss from 'postcss-scss'

const { NODE_ENV } = process.env

/**
* PostCSS config
*
* @param {import('postcss-load-config').ConfigContext} [ctx] - Context options
* @returns {import('postcss-load-config').Config} PostCSS Config
*/
export default ({ to = '' } = {}) => ({
export default ({ env = NODE_ENV, to = '' } = {}) => ({
plugins: [
// Add vendor prefixes
autoprefixer({ env: 'stylesheets' }),

// Add GOV.UK Frontend release version
!isDev && {
['test', 'production'].includes(env) && {
postcssPlugin: 'govuk-frontend-version',
Declaration: {
// Find CSS declaration for version, update value
Expand Down
17 changes: 17 additions & 0 deletions packages/govuk-frontend/postcss.config.unit.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,23 @@ describe('PostCSS config', () => {
})

describe('Plugins', () => {
describe('GOV.UK Frontend version', () => {
it('Skips version number for NODE_ENV=development', () => {
const config = configFn({ env: 'development' })
expect(getPluginNames(config)).not.toContain('govuk-frontend-version')
})

it('Adds version number for NODE_ENV=production', () => {
const config = configFn({ env: 'production' })
expect(getPluginNames(config)).toContain('govuk-frontend-version')
})

it('Adds version number for NODE_ENV=test', () => {
const config = configFn({ env: 'test' })
expect(getPluginNames(config)).toContain('govuk-frontend-version')
})
})

describe('CSS syntax parser', () => {
it.each([
{
Expand Down
7 changes: 0 additions & 7 deletions shared/tasks/helpers/task-arguments.mjs

This file was deleted.

63 changes: 0 additions & 63 deletions shared/tasks/helpers/task-arguments.unit.test.mjs

This file was deleted.

4 changes: 2 additions & 2 deletions shared/tasks/npm.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { paths } from '@govuk-frontend/config'
import runScript from '@npmcli/run-script'
import PluginError from 'plugin-error'

import { isDev } from './helpers/task-arguments.mjs'
import { task } from './index.mjs'

/**
Expand Down Expand Up @@ -31,7 +30,8 @@ export async function run(name, args = [], options) {
} catch (cause) {
const error = new Error(`Task for npm script '${name}' failed`, { cause })

if (!isDev) {
// Skip errors by default to allow Gulp to resume tasks
if (['test', 'production'].includes(process.env.NODE_ENV)) {
throw new PluginError(`npm run ${name}`, error, {
showProperties: false,
showStack: false
Expand Down

0 comments on commit 49c229e

Please sign in to comment.