Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions shared/tasks/assets.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { mkdir, writeFile } from 'fs/promises'
import { dirname } from 'path'
import { dirname, relative } from 'path'

/**
* Write asset helper
Expand All @@ -14,15 +14,27 @@ export async function write (filePath, result) {
const writeTasks = []

// Files to write
/** @type {SourceMap | undefined} */
const map = result.map ? JSON.parse(result.map.toString()) : undefined
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need map.sources a few lines down so parse to SourceMap here

Otherwise across Terser, Rollup and PostCSS we see source maps as:

  1. JavaScript object types that need JSON.stringify()
  2. Class instances with .toJSON() and .toString() methods
  3. Preprepared string type
  4. Missing undefined type

This can be seen via the AssetOutput @typedef in this file

const code = 'css' in result ? result.css : result.code
const map = result.map?.toString()

// 1. Write code (example.js)
writeTasks.push(writeFile(filePath, code))

// 2. Write source map (example.js.map)
if (map) {
writeTasks.push(writeFile(`${filePath}.map`, map))
if (map && 'sources' in map) {
map.sources = map.sources

/**
* Make source file:// paths relative (e.g. for Dart Sass)
* {@link https://sass-lang.com/documentation/js-api/interfaces/CompileResult#sourceMap}
*/
.map((path) => path.startsWith('file://')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PostCSS adds the Dart Sass entry point ../src/all.scss as a relative path

But all the other Dart Sass sources are absolute path file:// URLs

? relative(dirname(filePath), new URL(path).pathname)
: path
)

writeTasks.push(writeFile(`${filePath}.map`, JSON.stringify(map)))
}

await Promise.all(writeTasks)
Expand Down Expand Up @@ -70,3 +82,11 @@ export async function write (filePath, result) {
*
* @typedef {import('rollup').OutputChunk | import('terser').MinifyOutput | import('postcss').Result} AssetOutput
*/

/**
* Asset source maps
*
* {@link https://github.com/source-map/source-map-spec}
*
* @typedef {import('@jridgewell/source-map').DecodedSourceMap | import('@jridgewell/source-map').EncodedSourceMap} SourceMap
*/
45 changes: 45 additions & 0 deletions shared/tasks/build/dist.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ describe('dist/', () => {
})
})

describe('govuk-frontend-[version].min.css.map', () => {
let filename
let sourcemap

beforeAll(async () => {
filename = `govuk-frontend-${pkg.version}.min.css.map`
sourcemap = JSON.parse(await readFile(join(paths.dist, filename), 'utf8'))
})

it('should contain relative paths to sources', () => {
expect(sourcemap.sources).toContain('../src/govuk/all.scss')
expect(sourcemap.sources).toContain('../src/govuk/core/_govuk-frontend-version.scss')
})
})

describe('govuk-frontend-ie8-[version].min.css', () => {
let filename
let stylesheet
Expand All @@ -64,6 +79,21 @@ describe('dist/', () => {
})
})

describe('govuk-frontend-ie8-[version].min.css.map', () => {
let filename
let sourcemap

beforeAll(async () => {
filename = `govuk-frontend-ie8-${pkg.version}.min.css.map`
sourcemap = JSON.parse(await readFile(join(paths.dist, filename), 'utf8'))
})

it('should contain relative paths to sources', () => {
expect(sourcemap.sources).toContain('../src/govuk/all-ie8.scss')
expect(sourcemap.sources).toContain('../src/govuk/core/_govuk-frontend-version.scss')
})
})

describe('govuk-frontend-[version].min.js', () => {
let filename
let javascript
Expand All @@ -86,6 +116,21 @@ describe('dist/', () => {
})
})

describe('govuk-frontend-[version].min.js.map', () => {
let filename
let sourcemap

beforeAll(async () => {
filename = `govuk-frontend-${pkg.version}.min.js.map`
sourcemap = JSON.parse(await readFile(join(paths.dist, filename), 'utf8'))
})

it('should contain relative paths to sources', () => {
expect(sourcemap.sources).toContain('../src/govuk/all.mjs')
expect(sourcemap.sources).toContain('../src/govuk/common/govuk-frontend-version.mjs')
})
})

describe('VERSION.txt', () => {
let filename
let version
Expand Down
6 changes: 6 additions & 0 deletions shared/tasks/build/package.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ describe('package/', () => {
return output
}))

// Add Autoprefixer prefixes to all source '*.scss' files
.flatMap(mapPathTo(['**/*.scss'], ({ dir: requirePath, name }) => [
join(requirePath, `${name}.scss`),
join(requirePath, `${name}.scss.map`) // with source map
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Service teams with source maps enabled will have been inspecting our Autoprefixer-transformed sources, not the original prefix-free *.scss source files

]))

// Replaces source component '*.yaml' with:
// - `fixtures.json` fixtures for tests
// - `macro-options.json` component options
Expand Down
25 changes: 19 additions & 6 deletions shared/tasks/styles.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,25 @@ export async function compileStylesheet ([modulePath, { srcPath, destPath, fileP
let css
let map

// Configure PostCSS
/**
* Configure PostCSS
*
* @type {import('postcss').ProcessOptions}
*/
const options = {
from: moduleSrcPath,
to: moduleDestPath
to: moduleDestPath,

/**
* Always generate source maps for either:
*
* 1. PostCSS on Sass compiler result
* 2. PostCSS on Sass sources (Autoprefixer only)
*/
map: {
annotation: true,
inline: false
}
}

// Compile Sass to CSS
Expand Down Expand Up @@ -79,10 +94,8 @@ export async function compileStylesheet ([modulePath, { srcPath, destPath, fileP
}))

// Pass source maps to PostCSS
options.map = {
annotation: true,
inline: false,
prev: map
if (typeof options.map === 'object') {
options.map.prev = map
Copy link
Contributor Author

@colinrotherham colinrotherham Apr 20, 2023

Choose a reason for hiding this comment

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

PostCSS will automatically discover source maps via sourceMappingURL comments but we prefer to pass them directly from Dart Sass here (like we did previously)

}
}

Expand Down