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

Build tasks: Fix various JSDoc types #3380

Merged
merged 7 commits into from
Mar 17, 2023
Merged

Build tasks: Fix various JSDoc types #3380

merged 7 commits into from
Mar 17, 2023

Conversation

colinrotherham
Copy link
Contributor

This PR fixes various JSDoc types that have drifted due to:

  1. Dependabot updates (type changes were missed)
  2. Build tool changes (e.g. node-sass now removed)
  3. Build tool files and tests excluded from tsconfig.json

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3380 March 13, 2023 13:05 Inactive
@@ -14,7 +14,7 @@ export async function writeAsset (filePath, result) {
const writeTasks = []

// Files to write
const code = result.code || result.css?.toString()
const code = 'css' in result ? result.css : result.code
Copy link
Contributor Author

@colinrotherham colinrotherham Mar 13, 2023

Choose a reason for hiding this comment

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

Since Dart Sass landed in #3164 so result.css no longer needs toString()

@@ -49,5 +49,5 @@ export async function writeAsset (filePath, result) {
* 2. Terser minified bundle
* 3. Sass compiler result
*
* @typedef {import('rollup').OutputChunk | import('terser').MinifyOutput | import('node-sass').Result} AssetOutput
* @typedef {import('rollup').OutputChunk | import('terser').MinifyOutput | import('postcss').Result} AssetOutput
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missed reference to node-sass which should be the PostCSS transform result

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3380 March 13, 2023 13:09 Inactive
@colinrotherham colinrotherham changed the title Fix JSDoc types in build tasks Build tasks: Fix various JSDoc types Mar 13, 2023
@@ -111,7 +111,8 @@
"stylelint-config-gds": "^0.2.0",
"stylelint-order": "^6.0.2",
"typed-query-selector": "^2.9.1",
"typescript": "^4.9.5"
"typescript": "^4.9.5",
"vinyl": "^3.0.0"
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 removed this one but forgot we still type component data with it:

/**
 * @param {import('vinyl')} file
 * @returns {Promise<import('vinyl')>}
 */

@colinrotherham colinrotherham changed the base branch from main to build-paths March 15, 2023 09:26
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3380 March 15, 2023 09:26 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3380 March 15, 2023 10:11 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3380 March 17, 2023 11:19 Inactive
@@ -170,7 +170,7 @@ async function generateMacroOptions (file) {
* Parse YAML file content as JavaScript
*
* @param {import('vinyl')} file - Component data ${componentName}.yaml
* @returns {Promise<{ examples?: unknown[]; params?: unknown[] }>} Component options
* @returns {Promise<{ examples?: Record<string, unknown>[]; params?: Record<string, unknown>[] }>} Component options
Copy link
Member

@romaricpascal romaricpascal Mar 17, 2023

Choose a reason for hiding this comment

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

question Likely non blocking, but what does the Record type represent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a utility type for objects that are always keyed with the same name/value pair

Copy link
Member

Choose a reason for hiding this comment

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

Oh didn't think to look in the built-in types, cheers 😄

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3380 March 17, 2023 12:19 Inactive
Base automatically changed from build-paths to main March 17, 2023 13:15
@colinrotherham colinrotherham merged commit 207fc7d into main Mar 17, 2023
@colinrotherham colinrotherham deleted the build-task-types branch March 17, 2023 13:19
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