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

Replace metalsmith-tagcleaner with a custom marked renderer #2276

Merged
merged 3 commits into from
Aug 1, 2022

Conversation

domoscargin
Copy link
Contributor

@domoscargin domoscargin commented Jul 26, 2022

What

Replaces metalsmith-tagcleaner with a custom marked renderer which we pass to @metalsmith/in-place during conversion of our .njk files to HTML.

Why

metalsmith-tagcleaner is an unmaintained metalsmith plugin to remove <p> tags from around <img> tags. It runs after we've generated our HTML from our markdown and Nunjucks files.

In addition to a coupla NPM audit warnings, it recently started giving warnings during npm install:

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'metalsmith-tagcleaner@0.0.2',
npm WARN EBADENGINE   required: { node: '>=4.00' },
npm WARN EBADENGINE   current: { node: 'v16.14.2', npm: '8.5.0' }
npm WARN EBADENGINE }

How

We process our .njk files into HTML using @metalsmith/in-place. This relies on jstransformers under the hood, specifically jstransformer-marked, which allows us to pass in the normal marked options.

This means rather than loop over our HTML files checking the contents after they've been generated, we can instead use a custom marked renderer and avoid wrapping the images while the files are being converted. This should net us a small performance benefit.

Other notes

I've also bumped marked and both jstransformer packages. Think it makes sense to include them in this PR as they're what we're using under the hood. Changelogs are in the commit.

@netlify
Copy link

netlify bot commented Jul 26, 2022

You can preview this change here:

Name Link
🔨 Latest commit eb58287
🔍 Latest deploy log https://app.netlify.com/sites/govuk-design-system-preview/deploys/62e7a0fc96d30b000849c45b
😎 Deploy Preview https://deploy-preview-2276--govuk-design-system-preview.netlify.app/community/design-system-working-group
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@domoscargin domoscargin force-pushed the bk-spike-tagcleaner-replacement branch from 5a80940 to d33e4d5 Compare July 27, 2022 21:22
Makes sense to bump these as part of this work, since they're being used under the hood to transform our files.

jstransformer-nunjucks Changelog: https://github.com/jstransformers/jstransformer-nunjucks/blob/master/CHANGELOG.md

- The major version bump simply adds the option to use custom Nunjucks loaders

jstransformer-marked Changelog: https://github.com/jstransformers/jstransformer-marked/blob/master/HISTORY.md
@domoscargin domoscargin force-pushed the bk-spike-tagcleaner-replacement branch from e5d8ce0 to 2100ebd Compare July 27, 2022 21:43
@domoscargin domoscargin marked this pull request as ready for review July 27, 2022 21:44
@domoscargin domoscargin requested a review from a team July 28, 2022 09:21
lib/marked-renderer.js Outdated Show resolved Hide resolved
This better matches how things are done in Marked, and allows us to user `super` for cases other than images.

Co-authored-by: Oliver Byford <oliver.byford@digital.cabinet-office.gov.uk>
@domoscargin domoscargin force-pushed the bk-spike-tagcleaner-replacement branch from 8551f82 to eb58287 Compare August 1, 2022 09:46
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

👍🏻

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.

2 participants