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

Read component data from built fixtures.json #4042

Merged
merged 4 commits into from
Aug 14, 2023
Merged

Conversation

colinrotherham
Copy link
Contributor

We currently build two JSON files for the GOV.UK Design System

These are generated from component YAML files but aren't currently tested:

  • fixtures.json from ${componentName}.yaml
  • macro-options.json from ${componentName}.yaml

This PR updates all "component data" helpers to read from fixtures.json instead of YAML

The Review app no longer needs to access govuk-frontend/src YAML files

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4042 August 4, 2023 08:20 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4042 August 4, 2023 15:29 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4042 August 8, 2023 13:39 Inactive
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.

Looks neat, just need to confirm why we've moved to reading the list of components from dist rather than src (as it's a bit of a hidden requirement for that function that you need to have built the package before it can be run and may cause some confusion).

@@ -124,7 +124,7 @@ const getComponentFiles = (componentName = '') =>
getListing(
join(
packageNameToPath('govuk-frontend'),
`src/govuk/components/${componentName}/**/*`
`dist/govuk/components/${componentName}/**/*`
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm: are we're adding the requirement of reading from dist rather than src to ensure we read the list that's inside the package itself (and avoid potential missing components from older versions)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's the one, added an example in #4042 (comment)

@@ -94,7 +94,7 @@ params:

examples:
- name: default
data:
options:
Copy link
Member

Choose a reason for hiding this comment

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

Makes sense given we talk of macro "options" all around 🙌🏻

@colinrotherham
Copy link
Contributor Author

Thanks @romaricpascal

Looks neat, just need to confirm why we've moved to reading the list of components from dist rather than src (as it's a bit of a hidden requirement for that function that you need to have built the package before it can be run and may cause some confusion).

Yeah it was one of your future suggestions from #3498 (comment)

So for now, I think we can live with loading the YAML files from src. Might be worth using fixtures.json in the future though as it contains the pre-rendered examples (would need to check that they're all there though), but we can leave it out for now.

It came up again whilst looking at stats for legacy releases, for example:

  1. Checkout main with Exit this page and Task list components
  2. Install a legacy version for stats, e.g. GOV.UK Frontend v4.6.0

But you'll notice that our helpers:

  • List Exit this page and Task list for components
  • List Exit this page for componentsWithJavaScript

We were looping directories in packages/govuk-frontend/src not the locally installed version

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.

Good to go! ⛵

@colinrotherham colinrotherham merged commit 08c8478 into main Aug 14, 2023
@colinrotherham colinrotherham deleted the component-fixtures branch August 14, 2023 11:45
colinrotherham added a commit that referenced this pull request Aug 22, 2023
Read component data from built `fixtures.json`
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