USWDS-Site - Feature: Run prettier on accessibility tests, yml, and more #3120

mejiaj · 2025-02-20T17:12:21Z

Summary

Additional prettier scripts to ensure we're using the correct formatting in more than just SCSS.

This feature adds check and fix scripts for:

Accessibility test markdown
YML data
JS

Related issue

Closes #2868.

Problem statement

We're not running formatting checks on changelogs and a11y test findings. This causes inconsistencies and potential issues as we continue to develop.

Solution

This work adds:

New prettier scripts. Updated NPM scripts to run and fix prettier on many more files, like js, markdown¹, and yml.
Renamed prettier script. The previous prettier:scss script to check for errors. The old one has been renamed to prettier:fix:scss for consistency.
New CI check. There's a new check [lint_and_format] that runs before build to check for formatting and linting errors.
Fixed formatting issues. Issues flagged for YML and accessibility tests have been fixed.

Major changes

1. New NPM scripts

Clearly separated scripts that check vs those that fix. I've also added concurrently to fix and check for issues. The check does not run in parallel so we can clearly distinguish the different types of issues.

uswds-site/package.json

Lines 62 to 70 in fbe41ee

    
           "prettier:js": "npx prettier --check './**/*.js'", 
        
           "prettier:md": "npx prettier --check './**/accessibility-tests.md'", 
        
           "prettier:yml": "npx prettier --check './_data/**/*.yml'", 
        
           "prettier:fix:scss": "npx prettier --write './css/**/*.scss'", 
        
           "prettier:fix:js": "npx prettier --write './**/*.js'", 
        
           "prettier:fix:md": "npx prettier --write './**/accessibility-tests.md'", 
        
           "prettier:fix:yml": "npx prettier --write './_data/**/*.yml'", 
        
           "prettier:fix": "concurrently 'npm run prettier:fix:scss' 'npm run prettier:fix:md' 'npm run prettier:fix:yml'", 
        
           "prettier": "concurrently --maxProcesses 1 'npm run prettier:scss' 'npm run prettier:md' 'npm run prettier:yml'"

The general prettier script is also run in CI.

2. New CI check

Running general prettier check in CI now so that we can resolve issues before merge. This job is separated from the others because it'd be nice to know as soon as possible rather than waiting until all other jobs have finished.

Separating also helps us clearly see the issue and migrate it more easily to GitHub actions in the future.

uswds-site/.circleci/config.yml

Lines 146 to 148 in fbe41ee

    
           - lint_and_format: 
        
               requires: 
        
                 - install_node_deps

3. Running prettier on a11y tests

The issue says run prettier on markdown. But I've had to scope it down to only a11y tests, which was the main reason for creating the issue, due to regressions to markdown files that heavily rely on includes or adding classes.

Example of class issue
There are incompatibilities with the markdown formatter that Jekyll uses (kramdown) and the one Prettier is written for (CommonMark).

Issue

<!-- pages/patterns/select-language/two-languages.md -->

{:.site-component-section-title}

## Guidance

Because we use so much of this pattern and other possible regressions, I'm limiting the scope of the markdown checks.

Before	After

Workaround

<!-- pages/patterns/select-language/two-languages.md -->

<!-- prettier-ignore-start -->
## About this pattern
{:.site-component-section-title}
<!-- prettier-ignore-end -->

https://github.com/uswds/uswds-site/blob/fbe41ee6ae694439ebeca7c1b1a23bda83e0ebbb/.prettierignore#L21-L24

Summary
This is just one issue to illustrate the point. There are workarounds, but that increases the LOE to moderate or higher. In all, it would flag over 300 markdown formatting issues. Running the fix solves the prettier errors, but causes visual regressions that need to be carefully reviewed.

Testing and review

Ensure the scripts to check run successfully
- prettier:scss
- prettier:js. This will flag errors, but an optional test for now.
- prettier:md. Scoped to a11y tests until we can break down and address other markdown issues.
- prettier:yml. Checks against jekyll data, but could be extended to all YML files in the future.
- prettier. This will check SCSS, MD, and YML one at a time.
Ensure prettier runs on active development. Running npm start will now check for formatting issues, so we can catch during active development.
Ensure the CI task runs successfully. Confirm that CI checks for linting and formattingn in SCSS, MD (a11y tests), and YML.
- CI before formatting fixes - Should fail
- CI after fixes - Should pass

Markdown prettier check is limited to accessibility tests to avoid visual regressions in other pages. ↩

Limiting markdown checks to accessibility tests for now.

Data is used in `/documentation/settings` page

mejiaj · 2025-02-20T19:42:17Z

.prettierignore

Tip
For now, only running on accessibility tests. Need to scope out work to get all markdown files formatted without visual regressions.

mejiaj · 2025-02-20T19:42:53Z

.circleci/config.yml

Tip
Running separately gives us faster feedback and allows us to clearly see formatting failures.

mejiaj · 2025-02-20T19:46:12Z

_data/changelogs/component-memorable-date.yml

+    summary: Updated package name to `form-controls`.
+    affectsGuidance: true
+    githubPr: 1497
+    githubRepo: uswds-site


Tip
There shouldn't be any regressions in the memorable date changelog.
Preview link →

mejiaj · 2025-02-20T19:46:47Z

_data/changelogs/component-prose.yml

Preview link →

mejiaj · 2025-02-20T19:47:26Z

_data/changelogs/component-radio-buttons.yml

Preview →

mejiaj · 2025-02-20T19:48:22Z

_data/changelogs/component-range-slider.yml

Preview →

mejiaj · 2025-02-20T19:48:46Z

_data/changelogs/component-select.yml

Preview →

mejiaj · 2025-02-20T19:49:57Z

_data/settings/components/checkbox.yml

Tip
Changes to settings/components can be verified in the settings page.

mejiaj · 2025-02-20T19:52:12Z

package.json

+    "pa11y-ci:sitemap-json": "pa11y-ci --json > pa11y-results.json --sitemap http://localhost:4000/sitemap.xml --sitemap-exclude '/*.pdf|next|together/'",
+    "prettier:scss": "npx prettier --check './css/**/*.scss'",
+    "prettier:js": "npx prettier --check './**/*.js'",
+    "prettier:md": "npx prettier --check './**/accessibility-tests.md'",


Tip
Intentionally leaving this specific to accessibility-tests until we can scope out the work to tackle all markdown files.

amyleadem

Looking good! I had just a few questions about the package.json scripts. Not sure any action is needed, just curious to hear why you set it up that way.

amyleadem · 2025-02-20T20:10:43Z

package.json

+    "prettier:fix": "concurrently 'npm run prettier:fix:scss' 'npm run prettier:fix:md' 'npm run prettier:fix:yml'",
+    "prettier": "concurrently --maxProcesses 1 'npm run prettier:scss' 'npm run prettier:md' 'npm run prettier:yml'"


question: Why are prettier:js and prettier:fix:js excluded from these bulk prettier scans?

There's additional dependency and work required to work with eslint¹. Reverting previous commit [3463c1a] of running prettier because of eslint conflicts.

Footnotes

https://prettier.io/docs/integrating-with-linters#notes ↩

package.json

amyleadem · 2025-02-20T22:53:47Z

package.json

+    "prettier:md": "npx prettier --check './**/accessibility-tests.md'",
+    "prettier:yml": "npx prettier --check './_data/**/*.yml'",
+    "prettier:fix:scss": "npx prettier --write './css/**/*.scss'",
+    "prettier:fix:js": "npx prettier --write './**/*.js'",


issue: Running npm run prettier:fix:js returned some changed files. Can we include those updates in this PR?

question: Are there any conflicts with prettier:fix and our JS linting?

This is slightly out of scope. I'm including because it's a small change, but in the future want to keep the PR focused on original issue.

Fixed in 21a1fa8.

It does look like there is a conflict between prettier and linting for JS files. I see this error now during linting:

/Users/amy.leadem/Dev/github/workspace/uswds-site/js/sidenav.js 19:53 error A space is required after '[' computed-property-spacing 19:55 error A space is required before ']' computed-property-spacing /Users/amy.leadem/Dev/github/workspace/uswds-site/js/start.js 13:32 error A space is required after '[' computed-property-spacing 13:34 error A space is required before ']' computed-property-spacing ✖ 4 problems (4 errors, 0 warnings) 4 errors and 0 warnings potentially fixable with the `--fix` option.

Also noting that linting errors translate to passing checks in circleCI.

Reverting in 3463c1a. This is outside of current scope.

@mejiaj should the lint error there have flagged a circleCI failure? I'm a bit concerned that linting will be able to fail silently.

mejiaj · 2025-02-21T14:39:11Z

@amyleadem thanks for the review, ready for another look.

amyleadem

@mejiaj Found a couple more things for your review.

It looks like JS linting and prettier are in conflict, and it also looks like lint errors in CircleCI don't trigger the lint check to fail (See related comment for details).
It looks like the lint script is a bit muddled because it runs prettier inside of it. Wondering if we can keep those separate. (See comment below)

Let me know if you have questions.

.circleci/config.yml

This reverts commit 21a1fa8.

amyleadem

Looking good, but before I approve I am curious to hear your thoughts on the silent lint failures in circleCI.

James Mejia added 8 commits February 20, 2025 11:03

Add prettier scripts for checking, writing, and active development

166634a

Limiting markdown checks to accessibility tests for now.

Add lint and format check to CI

b5659aa

Format token data and fix typo in measure

3a83539

Format component settings data

bbef115

Data is used in `/documentation/settings` page

Format data for component a11y issues

7a51fdf

Format data for changelogs

c64635b

Format data for component lifecycle, migration, and monthly calls

8ef2d52

Format accessibility tests data and markdown

fbe41ee

mejiaj mentioned this pull request Feb 20, 2025

USWDS-Site - Prettier should run on .md files. #3037

Closed

mejiaj marked this pull request as ready for review February 20, 2025 19:40

mejiaj requested review from cathybaptista, mahoneycm, amyleadem and mlloydbixal February 20, 2025 19:40