From 039fe4c3cfc2b4c6efc45242bb6f3e894352ec19 Mon Sep 17 00:00:00 2001 From: Ward Peeters Date: Tue, 9 Mar 2021 11:03:47 +0100 Subject: [PATCH] update --- packages/gatsby-plugin-sitemap/package.json | 2 +- .../__snapshots__/options-validation.js.snap | 18 ------- .../src/__tests__/gatsby-node.js | 12 ++--- .../src/__tests__/options-validation.js | 52 ++++++++++++++----- .../gatsby-plugin-sitemap/src/gatsby-node.js | 22 +++++--- .../gatsby-plugin-sitemap/src/internals.js | 23 ++++---- .../src/options-validation.js | 20 +++---- yarn.lock | 11 ---- 8 files changed, 83 insertions(+), 77 deletions(-) delete mode 100644 packages/gatsby-plugin-sitemap/src/__tests__/__snapshots__/options-validation.js.snap diff --git a/packages/gatsby-plugin-sitemap/package.json b/packages/gatsby-plugin-sitemap/package.json index 19b105ab80490..67c6dfd9c3147 100644 --- a/packages/gatsby-plugin-sitemap/package.json +++ b/packages/gatsby-plugin-sitemap/package.json @@ -17,7 +17,7 @@ "@babel/core": "^7.12.3", "babel-preset-gatsby-package": "^1.1.0-next.1", "cross-env": "^7.0.3", - "joi": "^17.3.0" + "gatsby-plugin-utils": "^1.1.0-next.1" }, "homepage": "https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-plugin-sitemap#readme", "keywords": [ diff --git a/packages/gatsby-plugin-sitemap/src/__tests__/__snapshots__/options-validation.js.snap b/packages/gatsby-plugin-sitemap/src/__tests__/__snapshots__/options-validation.js.snap deleted file mode 100644 index 0f907b98b467c..0000000000000 --- a/packages/gatsby-plugin-sitemap/src/__tests__/__snapshots__/options-validation.js.snap +++ /dev/null @@ -1,18 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`gatsby-plugin-sitemap: options-validation tests validateOptions creates correct defaults 1`] = ` -Object { - "createLinkInHead": true, - "entryLimit": 45000, - "excludes": Array [], - "filterPages": [Function], - "output": "/sitemap", - "query": "{ site { siteMetadata { siteUrl } } allSitePage { nodes { path } } }", - "resolvePagePath": [Function], - "resolvePages": [Function], - "resolveSiteUrl": [Function], - "serialize": [Function], -} -`; - -exports[`gatsby-plugin-sitemap: options-validation tests validateOptions errors on invalid options 1`] = `[ValidationError: "wrong" is not allowed]`; diff --git a/packages/gatsby-plugin-sitemap/src/__tests__/gatsby-node.js b/packages/gatsby-plugin-sitemap/src/__tests__/gatsby-node.js index 75b54bbc3a527..ecf5877737966 100644 --- a/packages/gatsby-plugin-sitemap/src/__tests__/gatsby-node.js +++ b/packages/gatsby-plugin-sitemap/src/__tests__/gatsby-node.js @@ -1,15 +1,15 @@ +import path from "path" +import sitemap from "sitemap" +import { onPostBuild } from "../gatsby-node" +import { Joi } from "gatsby-plugin-utils" +import { pluginOptionsSchema } from "../options-validation" + jest.mock(`sitemap`, () => { return { simpleSitemapAndIndex: jest.fn(), } }) -const path = require(`path`) -const sitemap = require(`sitemap`) -const { onPostBuild } = require(`../gatsby-node`) -import { pluginOptionsSchema } from "../options-validation" -import Joi from "joi" - const schema = pluginOptionsSchema({ Joi }) const pathPrefix = `` diff --git a/packages/gatsby-plugin-sitemap/src/__tests__/options-validation.js b/packages/gatsby-plugin-sitemap/src/__tests__/options-validation.js index 0c7de4dd7c207..076b3f2df127a 100644 --- a/packages/gatsby-plugin-sitemap/src/__tests__/options-validation.js +++ b/packages/gatsby-plugin-sitemap/src/__tests__/options-validation.js @@ -1,22 +1,46 @@ import { pluginOptionsSchema } from "../options-validation" -import Joi from "joi" +import { testPluginOptionsSchema, Joi } from "gatsby-plugin-utils" -const schema = pluginOptionsSchema({ Joi }) +describe(`pluginOptionsSchema`, () => { + it(`should provide meaningful errors when fields are invalid`, async () => { + const expectedErrors = [`"wrong" is not allowed`] -describe(`gatsby-plugin-sitemap: options-validation tests`, () => { - describe(`validateOptions`, () => { - it(`creates correct defaults`, async () => { - const pluginOptions = await schema.validateAsync({}) - - expect(pluginOptions).toMatchSnapshot() + const { errors } = await testPluginOptionsSchema(pluginOptionsSchema, { + wrong: `test`, }) - it(`errors on invalid options`, async () => { - try { - await schema.validateAsync({ wrong: `test` }) - } catch (error) { - expect(error).toMatchSnapshot() + expect(errors).toEqual(expectedErrors) + }) + + it(`creates correct defaults`, async () => { + const pluginOptions = await pluginOptionsSchema({ Joi }).validateAsync({}) + + expect(pluginOptions).toMatchInlineSnapshot(` + Object { + "createLinkInHead": true, + "entryLimit": 45000, + "excludes": Array [], + "filterPages": [Function], + "output": "/sitemap", + "query": "{ site { siteMetadata { siteUrl } } allSitePage { nodes { path } } }", + "resolvePagePath": [Function], + "resolvePages": [Function], + "resolveSiteUrl": [Function], + "serialize": [Function], } - }) + `) + }) + + it.each` + options + ${undefined} + ${{}} + `(`should validate the schema: $options`, async ({ options }) => { + const { isValid } = await testPluginOptionsSchema( + pluginOptionsSchema, + options + ) + + expect(isValid).toBe(true) }) }) diff --git a/packages/gatsby-plugin-sitemap/src/gatsby-node.js b/packages/gatsby-plugin-sitemap/src/gatsby-node.js index 8de7bfd975608..68f517d63b391 100644 --- a/packages/gatsby-plugin-sitemap/src/gatsby-node.js +++ b/packages/gatsby-plugin-sitemap/src/gatsby-node.js @@ -1,7 +1,7 @@ import path from "path" import { simpleSitemapAndIndex } from "sitemap" import { pluginOptionsSchema } from "./options-validation" -import { prefixPath, pageFilter, reporterPrefix } from "./internals" +import { prefixPath, pageFilter, REPORTER_PREFIX } from "./internals" exports.pluginOptionsSchema = pluginOptionsSchema @@ -22,28 +22,34 @@ exports.onPostBuild = async ( const { data: queryRecords } = await graphql(query) reporter.verbose( - `${reporterPrefix} Query Results:\n${JSON.stringify(queryRecords, null, 2)}` + `${REPORTER_PREFIX} Query Results:\n${JSON.stringify( + queryRecords, + null, + 2 + )}` ) // resolvePages and resolveSuteUrl are allowed to be sync or async. The Promise.resolve handles each possibility const allPages = await Promise.resolve( resolvePages(queryRecords) - ).catch(err => reporter.panic(`${reporterPrefix} Error resolving Pages`, err)) + ).catch(err => + reporter.panic(`${REPORTER_PREFIX} Error resolving Pages`, err) + ) const siteUrl = await Promise.resolve( resolveSiteUrl(queryRecords) ).catch(err => - reporter.panic(`${reporterPrefix} Error resolving Site URL`, err) + reporter.panic(`${REPORTER_PREFIX} Error resolving Site URL`, err) ) if (!Array.isArray(allPages)) { reporter.panic( - `${reporterPrefix} The \`resolvePages\` function did not return an array.` + `${REPORTER_PREFIX} The \`resolvePages\` function did not return an array.` ) } reporter.verbose( - `${reporterPrefix} Filtering ${allPages.length} pages based on ${excludes.length} excludes` + `${REPORTER_PREFIX} Filtering ${allPages.length} pages based on ${excludes.length} excludes` ) const { filteredPages, messages } = pageFilter( @@ -58,7 +64,7 @@ exports.onPostBuild = async ( messages.forEach(message => reporter.verbose(message)) reporter.verbose( - `${reporterPrefix} ${filteredPages.length} pages remain after filtering` + `${REPORTER_PREFIX} ${filteredPages.length} pages remain after filtering` ) const serializedPages = [] @@ -73,7 +79,7 @@ exports.onPostBuild = async ( ...rest, }) } catch (err) { - reporter.panic(`${reporterPrefix} Error serializing pages`, err) + reporter.panic(`${REPORTER_PREFIX} Error serializing pages`, err) } } diff --git a/packages/gatsby-plugin-sitemap/src/internals.js b/packages/gatsby-plugin-sitemap/src/internals.js index 18ccdb430a983..2a6af7b9f3ed4 100644 --- a/packages/gatsby-plugin-sitemap/src/internals.js +++ b/packages/gatsby-plugin-sitemap/src/internals.js @@ -1,6 +1,6 @@ import minimatch from "minimatch" -export const reporterPrefix = `[gatsby-plugin-sitemap]:` +export const REPORTER_PREFIX = `[gatsby-plugin-sitemap]:` /** * @@ -33,7 +33,7 @@ export function prefixPath({ url, siteUrl, pathPrefix = `` }) { export function resolveSiteUrl(data) { if (!data?.site?.siteMetadata?.siteUrl) { throw Error( - `\`siteUrl\` does not exist on \`siteMetadata\` in the data returned from the query. + `\`siteUrl\` does not exist on \`siteMetadata\` in the data returned from the query. Add this to your custom query or provide a custom \`resolveSiteUrl\` function. https://www.gatsbyjs.com/plugins/gatsby-plugin-sitemap/#api-reference ` @@ -55,7 +55,7 @@ export function resolveSiteUrl(data) { export function resolvePagePath(page) { if (!page?.path) { throw Error( - `\`path\` does not exist on your page object. + `\`path\` does not exist on your page object. Make the page URI available at \`path\` or provide a custom \`resolvePagePath\` function. https://www.gatsbyjs.com/plugins/gatsby-plugin-sitemap/#api-reference ` @@ -108,13 +108,15 @@ export function defaultFilterPages( ) { if (typeof excludedRoute !== `string`) { throw new Error( - `You've passed something other than string to the exclude array. This is supported, but you'll have to write a custom filter function. + `You've passed something other than string to the exclude array. This is supported, but you'll have to write a custom filter function. Ignoring the input for now: ${JSON.stringify(excludedRoute, null, 2)} https://www.gatsbyjs.com/plugins/gatsby-plugin-sitemap/#api-reference ` ) } + // Minimatch is always scary without an example + // TODO add example return minimatch( withoutTrailingSlash(resolvePagePath(page)), withoutTrailingSlash(excludedRoute) @@ -157,6 +159,7 @@ export function pageFilter({ allPages, filterPages, excludes }) { throw new Error(`Invalid options passed to page Filter function`) } + // TODO we should optimize these loops const filteredPages = allPages.filter(page => { const defaultFilterMatches = defaultExcludes.some((exclude, i, arr) => { try { @@ -167,17 +170,19 @@ export function pageFilter({ allPages, filterPages, excludes }) { }) // default excludes can only be found once, so remove them from the arr once excluded - if (doesMatch) arr.splice(i, 1) + if (doesMatch) { + arr.splice(i, 1) + } return doesMatch } catch { - throw new Error(`${reporterPrefix} Error in default page filter`) + throw new Error(`${REPORTER_PREFIX} Error in default page filter`) } }) if (defaultFilterMatches) { messages.push( - `${reporterPrefix} Default filter excluded page ${resolvePagePath( + `${REPORTER_PREFIX} Default filter excluded page ${resolvePagePath( page )}` ) @@ -197,7 +202,7 @@ export function pageFilter({ allPages, filterPages, excludes }) { }) } catch { throw new Error( - `${reporterPrefix} Error in custom page filter. + `${REPORTER_PREFIX} Error in custom page filter. If you've customized your excludes you may need to provide a custom "filterPages" function in your config. https://www.gatsbyjs.com/plugins/gatsby-plugin-sitemap/#api-reference ` @@ -207,7 +212,7 @@ export function pageFilter({ allPages, filterPages, excludes }) { if (customFilterMatches) { messages.push( - `${reporterPrefix} Custom filtering excluded page ${resolvePagePath( + `${REPORTER_PREFIX} Custom filtering excluded page ${resolvePagePath( page )}` ) diff --git a/packages/gatsby-plugin-sitemap/src/options-validation.js b/packages/gatsby-plugin-sitemap/src/options-validation.js index 9eae0fb889835..f24dc6beec44f 100644 --- a/packages/gatsby-plugin-sitemap/src/options-validation.js +++ b/packages/gatsby-plugin-sitemap/src/options-validation.js @@ -56,12 +56,12 @@ export const pluginOptionsSchema = ({ Joi }) => }) .description( stripIndent` - (GraphQL Query) The query for the data you need to generate the sitemap. + (GraphQL Query) The query for the data you need to generate the sitemap. It's required to get the site's URL, if you are not fetching it from \`site.siteMetadata.siteUrl\`, - you will need to set a custom \`resolveSiteUrl\` function. - If you override the query, you may need to pass in a custom \`resolvePagePath\` or - \`resolvePages\` to keep everything working. - If you fetch pages without using \`allSitePage.nodes\` query structure + you will need to set a custom \`resolveSiteUrl\` function. + If you override the query, you may need to pass in a custom \`resolvePagePath\` or + \`resolvePages\` to keep everything working. + If you fetch pages without using \`allSitePage.nodes\` query structure you will definately need to customize the \`resolvePages\` function.` ), excludes: Joi.array() @@ -69,9 +69,9 @@ export const pluginOptionsSchema = ({ Joi }) => .default([]) .description( stripIndent` - An array of paths to exclude from the sitemap. - While this is usually an array of strings it is possible to - enter other data types into this array for custom filtering. + An array of paths to exclude from the sitemap. + While this is usually an array of strings it is possible to + enter other data types into this array for custom filtering. Doing so will require customization of the \`filterPages\` function.` ), resolveSiteUrl: Joi.function() @@ -92,8 +92,8 @@ export const pluginOptionsSchema = ({ Joi }) => filterPages: Joi.function() .default(() => defaultFilterPages) .description( - stripIndent`Takes the current page a string (or other object) - from the \`exclude\` array and expects a boolean to be returned. + stripIndent`Takes the current page a string (or other object) + from the \`exclude\` array and expects a boolean to be returned. \`true\` excludes the path, \`false\` keeps it.` ), serialize: Joi.function() diff --git a/yarn.lock b/yarn.lock index a9ea7f6336ddb..0c11a708c0228 100644 --- a/yarn.lock +++ b/yarn.lock @@ -15457,17 +15457,6 @@ joi@^17.2.1, joi@^17.4.0: "@sideway/formula" "^3.0.0" "@sideway/pinpoint" "^2.0.0" -joi@^17.3.0: - version "17.3.0" - resolved "https://registry.yarnpkg.com/joi/-/joi-17.3.0.tgz#f1be4a6ce29bc1716665819ac361dfa139fff5d2" - integrity sha512-Qh5gdU6niuYbUIUV5ejbsMiiFmBdw8Kcp8Buj2JntszCkCfxJ9Cz76OtHxOZMPXrt5810iDIXs+n1nNVoquHgg== - dependencies: - "@hapi/hoek" "^9.0.0" - "@hapi/topo" "^5.0.0" - "@sideway/address" "^4.1.0" - "@sideway/formula" "^3.0.0" - "@sideway/pinpoint" "^2.0.0" - jpeg-js@^0.4.0: version "0.4.1" resolved "https://registry.yarnpkg.com/jpeg-js/-/jpeg-js-0.4.1.tgz#937a3ae911eb6427f151760f8123f04c8bfe6ef7"