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

Add source maps to compiled JavaScript and CSS #3023

Merged
merged 3 commits into from
Nov 28, 2022
Merged

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Nov 18, 2022

This PR adds source maps to our compiled code

For example:

govuk-frontend-4.4.0.min.js

//# sourceMappingURL=govuk-frontend-4.4.0.min.js.map

govuk-frontend-4.4.0.min.css

/*# sourceMappingURL=govuk-frontend-4.4.0.min.css.map */

When browser developer tools are open the associated source map files are downloaded

JavaScript source maps

See log messages and stack traces mapped to the *.mjs source code (with line numbers)

JavaScript source maps

Click through to the original source via the browser

JavaScript source maps (code)

CSS source maps

See styles mapped to the Sass *.scss source code (with line numbers)

CSS source maps

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3023 November 18, 2022 15:16 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3023 November 21, 2022 18:14 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3023 November 21, 2022 18:18 Inactive
@romaricpascal
Copy link
Member

Sourcemaps are great and the PR looks technically neat for adding them in. Just looking into what will consume them so we're clear on who will benefit and who will not (depending on how code is loaded) and maybe check if some guidance will need updating. More news on this space in a bit 😄

@romaricpascal
Copy link
Member

romaricpascal commented Nov 23, 2022

Who benefits from sourcemaps?

TL;DR:

  • People using dist will benefit from a guidance update to copy the files alongside the .min.{js,css} files.
  • Services statically serving node_modules/govuk-frontend[/govuk] will serve the sourcemaps automatically.
  • Prototype Kit and services statically serving individual files (like all.js) will need to also statically serve the .map file.
  • People using bundlers will need to add plugins/loaders to read the sourcemaps, unless they're using Babel or loading our ESM files (which they may well be doing if using Babel anyway).
  • Scss builds from source so sourcemaps don't need reading.
  • PostCSS will automatically consume sourcemaps.

People loading the files from dist on their page

Instead of looking at a cryptic file when an error occurs in the console, people using the release archive will get more readable code which may help us with support.

For that to work, though, we'll need to update the installation instructions to tell people to not only copy the .js and .css files, but also the sourcemaps.

People using the npm package

Serving files from node_modules with a static middleware

If they serve node_modules/govuk-frontend using something like Express's static, their server will make the sourcemaps available, so happy days.

If they point to individual files (eg. 'node_modules/govuk-frontend/govuk/all.js'), they'll need to also serve the .js.map file as well. For the prototype kit, for example, only the JS file will be served, so browsers won't be able to load the sourcemap.

Same situation would apply as well if people vendor their files in their framework of choice's public folder.

With bundlers

Bundlers outputing sourcemaps will be something developer of the actual project using govuk-frontend will need to enable (for ex. using the devtool option in Webpack or the sourcemap outputOption in Rollup). Up to them.

Outputing sourcemaps is only part of the job, though. By default bundlers don't consume sourcemaps from libraries, only create them from the modules they load. To consume them, people will need to add a plugin:

If Babel is involved for compiling the code during bundling, it is configured by default to consume sourcemaps

Note that if people load our ESM files, they'll technically be loading the sources, so won't need to consume sourcemaps.

If it's TypeScript consuming govuk-frontend, there's no support for input source maps yet. It follows Node's module resolution so I suspect it'll load ESM files directly.

With asset pipelines

For example Sprocket, used by Rails and Middleman (tech-docs-gem, for ex). Looks like support was added in v4, though unclear if it'd serve the sourcemaps alongside "bundling" the JS files.

Scss

Sass can output sourcemaps, and the projects would be building from our untouched Scss files so get pointed to the right place should they enable sourcemaps output. I think people could enable sourceMapIncludeSources if they were consuming our built all.css with Sass.

CSS

If people's project consume our CSS files, instead of SCSS files, through PostCSS for example. The later seems to automatically consume source maps

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3023 November 23, 2022 14:56 Inactive
@colinrotherham colinrotherham changed the base branch from main to gulp-plumber November 23, 2022 15:00
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3023 November 23, 2022 15:00 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3023 November 23, 2022 15:04 Inactive
@colinrotherham colinrotherham changed the base branch from gulp-plumber to gulp-plumber-rollup November 23, 2022 15:47
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3023 November 23, 2022 15:47 Inactive
@colinrotherham colinrotherham force-pushed the gulp-plumber-rollup branch 3 times, most recently from a93c678 to 72e56d9 Compare November 23, 2022 23:48
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3023 November 28, 2022 14:13 Inactive
@colinrotherham
Copy link
Contributor Author

@romaricpascal Rebased and ready for review again 👍

Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

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

Looking neat! 🙌🏻

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3023 November 28, 2022 14:45 Inactive
Base automatically changed from gulp-plumber-rollup to main November 28, 2022 17:26
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3023 November 28, 2022 17:32 Inactive
@colinrotherham colinrotherham merged commit b64d8aa into main Nov 28, 2022
@colinrotherham colinrotherham deleted the sourcemaps branch November 28, 2022 17:36
@colinrotherham colinrotherham changed the title Add source maps to compiled JavaScript + CSS Add source maps to compiled JavaScript and CSS Dec 9, 2022
colinrotherham added a commit that referenced this pull request Dec 9, 2022
colinrotherham added a commit that referenced this pull request Dec 12, 2022
@colinrotherham colinrotherham added this to the 4.5.0 milestone Jan 24, 2023
@owenatgov owenatgov mentioned this pull request Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants