Merge pull request github#24737 from github/repo-sync

repo sync

lib/frontmatter.js

@@ -1,5 +1,4 @@
 import parse from './read-frontmatter.js'
-import semver from 'semver'
 import { allVersions } from './all-versions.js'
 import { allTools } from './all-tools.js'
 import { getDeepDataByLanguage } from './get-data.js'
@@ -16,10 +15,12 @@ const layoutNames = [
 const guideTypes = ['overview', 'quick_start', 'tutorial', 'how_to', 'reference']
 
 export const schema = {
+  type: 'object',
+  required: ['title', 'versions'],
+  additionalProperties: false,
   properties: {
     title: {
       type: 'string',
-      required: true,
       translatable: true,
     },
     shortTitle: {
@@ -69,7 +70,7 @@ export const schema = {
     layout: {
       type: ['string', 'boolean'],
       enum: layoutNames,
-      message: 'must be the filename of an existing layout file, or `false` for no layout',
+      errorMessage: 'must be the filename of an existing layout file, or `false` for no layout',
     },
     redirect_from: {
       type: 'array',
@@ -120,8 +121,12 @@ export const schema = {
           items: {
             type: 'object',
             properties: {
-              title: 'string',
-              href: 'string',
+              title: {
+                type: 'string',
+              },
+              href: {
+                type: 'string',
+              },
             },
           },
         },
@@ -170,8 +175,12 @@ export const schema = {
     communityRedirect: {
       type: 'object',
       properties: {
-        name: 'string',
-        href: 'string',
+        name: {
+          type: 'string',
+        },
+        href: {
+          type: 'string',
+        },
       },
     },
     // Platform-specific content preference
@@ -196,15 +205,16 @@ export const schema = {
     // External products specified on the homepage
     externalProducts: {
       type: 'object',
+      required: ['electron'],
       properties: {
         electron: {
           type: 'object',
-          required: true,
+          required: ['id', 'name', 'href', 'external'],
           properties: {
-            id: { type: 'string', required: true },
-            name: { type: 'string', required: true },
-            href: { type: 'string', format: 'url', required: true },
-            external: { type: 'boolean', required: true },
+            id: { type: 'string' },
+            name: { type: 'string' },
+            href: { type: 'string', format: 'url' },
+            external: { type: 'boolean' },
           },
         },
       },
@@ -252,20 +262,22 @@ const featureVersionsProp = {
     items: {
       type: 'string',
     },
-    message:
+    errorMessage:
       'must be the name (or names) of a feature that matches "filename" in data/features/_filename_.yml',
   },
 }
 
 const semverRange = {
   type: 'string',
-  conform: semver.validRange,
-  message: 'Must be a valid SemVer range',
+  format: 'semver',
+  // This is JSON pointer syntax with ajv so we can specify the bad version
+  // in the error message.
+  // eslint-disable-next-line no-template-curly-in-string
+  errorMessage: 'Must be a valid SemVer range: ${0}',
 }
 
 schema.properties.versions = {
   type: ['object', 'string'], // allow a '*' string to indicate all versions
-  required: true,
   additionalProperties: false, // don't allow any versions in FM that aren't defined in lib/all-versions
   properties: Object.values(allVersions).reduce((acc, versionObj) => {
     acc[versionObj.plan] = semverRange
@@ -277,8 +289,6 @@ schema.properties.versions = {
 export function frontmatter(markdown, opts = {}) {
   const defaults = {
     schema,
-    validateKeyNames: true,
-    validateKeyOrder: false, // TODO: enable this once we've sorted all the keys. See issue 9658
   }
 
   return parse(markdown, Object.assign({}, defaults, opts))

lib/read-frontmatter.js

@@ -1,9 +1,21 @@
 import matter from 'gray-matter'
-import revalidator from 'revalidator'
-import { difference, intersection } from 'lodash-es'
+import Ajv from 'ajv'
+import addErrors from 'ajv-errors'
+import addFormats from 'ajv-formats'
+import semver from 'semver'
 
-function readFrontmatter(markdown, opts = { validateKeyNames: false, validateKeyOrder: false }) {
-  const schema = opts.schema || { properties: {} }
+const ajv = new Ajv({ allErrors: true, allowUnionTypes: true })
+ajv.addKeyword({
+  keyword: 'translatable',
+})
+ajv.addFormat('semver', {
+  validate: (x) => semver.validRange(x),
+})
+addErrors(ajv)
+addFormats(ajv)
+
+function readFrontmatter(markdown, opts = {}) {
+  const schema = opts.schema || { type: 'object', properties: {} }
   const filepath = opts.filepath || null
 
   let content, data
@@ -33,40 +45,39 @@ function readFrontmatter(markdown, opts = { validateKeyNames: false, validateKey
     return { errors }
   }
 
-  const allowedKeys = Object.keys(schema.properties)
-  const existingKeys = Object.keys(data)
-  const expectedKeys = intersection(allowedKeys, existingKeys)
-
-  ;({ errors } = revalidator.validate(data, schema))
+  const ajvValidate = ajv.compile(schema)
+  const valid = ajvValidate(data)
 
-  // add filepath property to each error object
-  if (errors.length && filepath) {
-    errors = errors.map((error) => Object.assign(error, { filepath }))
+  if (!valid) {
+    errors = ajvValidate.errors
   }
 
-  // validate key names
-  if (opts.validateKeyNames) {
-    const invalidKeys = difference(existingKeys, allowedKeys)
-    invalidKeys.forEach((key) => {
-      const error = {
-        property: key,
-        message: `not allowed. Allowed properties are: ${allowedKeys.join(', ')}`,
-      }
-      if (filepath) error.filepath = filepath
-      errors.push(error)
-    })
+  // Combine the AJV-supplied `instancePath` and `params` into a more user-friendly frontmatter path.
+  // For example, given:
+  //   "instancePath": "/versions",
+  //   "params": { "additionalProperty": "ftp" }
+  // return:
+  //   property: 'versions.ftp'
+  //
+  // The purpose is to help users understand that the error is on the `fpt` key within the `versions` object.
+  // Note if the error is on a top-level FM property like `title`, the `instancePath` will be empty.
+  const cleanPropertyPath = (params, instancePath) => {
+    const mainProps = Object.values(params)[0]
+    if (!instancePath) return mainProps
+
+    const prefixProps = instancePath.replace('/', '').replace(/\//g, '.')
+    return typeof mainProps !== 'object' ? `${prefixProps}.${mainProps}` : prefixProps
   }
 
-  // validate key order
-  if (opts.validateKeyOrder && existingKeys.join('') !== expectedKeys.join('')) {
-    const error = {
-      property: 'keys',
-      message: `keys must be in order. Current: ${existingKeys.join(
-        ','
-      )}; Expected: ${expectedKeys.join(',')}`,
-    }
-    if (filepath) error.filepath = filepath
-    errors.push(error)
+  if (!valid && filepath) {
+    errors = ajvValidate.errors.map((error) => {
+      const userFriendly = {}
+      userFriendly.property = cleanPropertyPath(error.params, error.instancePath)
+      userFriendly.message = error.message
+      userFriendly.reason = error.keyword
+      userFriendly.filepath = filepath
+      return userFriendly
+    })
   }
 
   return { content, data, errors }

tests/helpers/schemas/feature-versions-schema.js

@@ -20,25 +20,4 @@ featureVersions.additionalProperties = false
 // avoid ajv strict warning
 featureVersions.type = 'object'
 
-// *** TODO: We can drop the following once the frontmatter.js schema has been updated to work with AJV. ***
-const properties = {}
-Object.keys(featureVersions.properties.versions.properties).forEach((key) => {
-  const value = Object.assign({}, featureVersions.properties.versions.properties[key])
-
-  // AJV supports errorMessage, not message.
-  value.errorMessage = value.message
-  delete value.message
-
-  // AJV doesn't support conform, so we'll add semver validation in the lint-versioning test.
-  if (value.conform) {
-    value.format = 'semver'
-    delete value.conform
-  }
-  properties[key] = value
-})
-
-featureVersions.properties.versions.properties = properties
-delete featureVersions.properties.versions.required
-// *** End TODO ***
-
 export default featureVersions

tests/helpers/schemas/learning-tracks-schema.js

@@ -4,27 +4,6 @@ import { schema } from '../../../lib/frontmatter.js'
 // so we can import that part of the FM schema.
 const versionsProps = Object.assign({}, schema.properties.versions)
 
-// Tweak the imported versions schema so it works with AJV.
-// *** TODO: We can drop the following once the frontmatter.js schema has been updated to work with AJV. ***
-const properties = {}
-Object.keys(versionsProps.properties).forEach((key) => {
-  const value = Object.assign({}, versionsProps.properties[key])
-
-  // AJV supports errorMessage, not message.
-  value.errorMessage = value.message
-  delete value.message
-
-  // AJV doesn't support conform, so we'll add semver validation in the lint-files test.
-  if (value.conform) {
-    value.format = 'semver'
-    delete value.conform
-  }
-  properties[key] = value
-})
-
-versionsProps.properties = properties
-// *** End TODO ***
-
 // `versions` are not required in learning tracks the way they are in FM.
 delete versionsProps.required
 

tests/helpers/schemas/secret-scanning-schema.js

@@ -4,28 +4,6 @@ import { schema } from '../../../lib/frontmatter.js'
 // so we can import that part of the FM schema.
 const versionsProps = Object.assign({}, schema.properties.versions)
 
-// Tweak the imported versions schema so it works with AJV.
-// *** TODO: We can drop the following once the frontmatter.js schema has been updated to work with AJV. ***
-const properties = {}
-Object.keys(versionsProps.properties).forEach((key) => {
-  const value = Object.assign({}, versionsProps.properties[key])
-
-  // AJV supports errorMessage, not message.
-  value.errorMessage = value.message
-  delete value.message
-
-  // AJV doesn't support conform, so we'll add semver validation in the lint-secret-scanning-data test.
-  if (value.conform) {
-    value.format = 'semver'
-    delete value.conform
-  }
-  properties[key] = value
-})
-
-versionsProps.properties = properties
-delete versionsProps.required
-// *** End TODO ***
-
 // The secret-scanning.json contains an array of objects that look like this:
 // {
 //   "provider": "Azure",