Skip to content

Commit

Permalink
chore(gatsby): move to latest joi (#29792)
Browse files Browse the repository at this point in the history
  • Loading branch information
wardpeet committed Mar 1, 2021
1 parent 2758329 commit 86b8b26
Show file tree
Hide file tree
Showing 13 changed files with 106 additions and 78 deletions.
13 changes: 4 additions & 9 deletions integration-tests/structured-logging/__tests__/to-do.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ const commonAssertions = events => {
const actionSchema = joi.alternatives().try(
joi
.object({
type: joi.string().required().valid([`SET_STATUS`]),
type: joi.string().required().valid(`SET_STATUS`),
// TODO: We should change this to always be an Object I think pieh
payload: joi
.string()
.required()
.valid([`SUCCESS`, `IN_PROGRESS`, `FAILED`, `INTERRUPTED`]),
.valid(`SUCCESS`, `IN_PROGRESS`, `FAILED`, `INTERRUPTED`),
// Should this be here or one level up?
timestamp: joi.string().required(),
})
Expand All @@ -121,12 +121,7 @@ const commonAssertions = events => {
type: joi
.string()
.required()
.valid([
`ACTIVITY_START`,
`ACTIVITY_UPDATE`,
`ACTIVITY_END`,
`LOG`,
]),
.valid(`ACTIVITY_START`, `ACTIVITY_UPDATE`, `ACTIVITY_END`, `LOG`),
payload: joi.object(),
// Should this be here or one level up?
timestamp: joi.string().required(),
Expand All @@ -135,7 +130,7 @@ const commonAssertions = events => {
)

const eventSchema = joi.object({
type: joi.string().required().valid([`LOG_ACTION`]),
type: joi.string().required().valid(`LOG_ACTION`),
action: actionSchema,
})
events.forEach(event => {
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/structured-logging/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"fs-extra": "^9.0.1",
"jest": "^24.0.0",
"jest-cli": "^24.0.0",
"joi": "^14.3.1",
"joi": "^17.4.0",
"lodash": "^4.17.20",
"node-fetch": "^2.6.1"
}
Expand Down
4 changes: 2 additions & 2 deletions packages/gatsby-cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
},
"dependencies": {
"@babel/code-frame": "^7.10.4",
"@hapi/joi": "^15.1.1",
"@types/common-tags": "^1.8.0",
"better-opn": "^2.0.0",
"chalk": "^4.1.0",
Expand All @@ -29,6 +28,7 @@
"gatsby-telemetry": "^2.1.0-next.1",
"hosted-git-info": "^3.0.6",
"is-valid-path": "^0.1.1",
"joi": "^17.4.0",
"lodash": "^4.17.20",
"meant": "^1.0.2",
"node-fetch": "^2.6.1",
Expand Down Expand Up @@ -101,4 +101,4 @@
"engines": {
"node": ">=12.13.0"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,26 @@
import { errorSchema } from "../error-schema"

test(`throws invalid on an invalid error`, () => {
expect(errorSchema.validate({ lol: `true` })).rejects.toBeDefined()
test(`returns invalid on an invalid error`, () => {
expect(errorSchema.validate({ lol: `true` })).toMatchInlineSnapshot(`
Object {
"error": [ValidationError: "lol" is not allowed],
"value": Object {
"lol": "true",
},
}
`)
})

test(`does not throw on a valid schema`, () => {
test(`returns a valid value`, () => {
expect(
errorSchema.validate({
context: {},
})
).resolves.toEqual(expect.any(Object))
).toMatchInlineSnapshot(`
Object {
"value": Object {
"context": Object {},
},
}
`)
})
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const constructError = (

// validate
const { error } = errorSchema.validate(structuredError)
if (error !== null) {
if (error) {
console.log(`Failed to validate error`, error)
process.exit(1)
}
Expand Down
8 changes: 4 additions & 4 deletions packages/gatsby-cli/src/structured-errors/error-schema.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Joi from "@hapi/joi"
import Joi from "joi"
import { ILocationPosition, IStructuredError } from "./types"

export const Position: Joi.ObjectSchema<ILocationPosition> = Joi.object().keys({
Expand All @@ -20,9 +20,9 @@ export const errorSchema: Joi.ObjectSchema<IStructuredError> = Joi.object().keys
})
)
.allow(null),
category: Joi.string().valid([`USER`, `SYSTEM`, `THIRD_PARTY`]),
level: Joi.string().valid([`ERROR`, `WARNING`, `INFO`, `DEBUG`]),
type: Joi.string().valid([`GRAPHQL`, `CONFIG`, `WEBPACK`, `PLUGIN`]),
category: Joi.string().valid(`USER`, `SYSTEM`, `THIRD_PARTY`),
level: Joi.string().valid(`ERROR`, `WARNING`, `INFO`, `DEBUG`),
type: Joi.string().valid(`GRAPHQL`, `CONFIG`, `WEBPACK`, `PLUGIN`),
filePath: Joi.string(),
location: Joi.object({
start: Position.required(),
Expand Down
2 changes: 0 additions & 2 deletions packages/gatsby/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"@babel/types": "^7.12.6",
"@gatsbyjs/reach-router": "^1.3.5",
"@gatsbyjs/webpack-hot-middleware": "^2.25.2",
"@hapi/joi": "^15.1.1",
"@mikaelkristiansson/domready": "^1.0.10",
"@nodelib/fs.walk": "^1.2.4",
"@pmmmwh/react-refresh-webpack-plugin": "^0.4.3",
Expand Down Expand Up @@ -170,7 +169,6 @@
"@babel/cli": "^7.12.1",
"@babel/runtime": "^7.12.5",
"@types/eslint": "^7.2.6",
"@types/hapi__joi": "^16.0.12",
"@types/micromatch": "^4.0.1",
"@types/normalize-path": "^3.0.0",
"@types/reach__router": "^1.3.5",
Expand Down

This file was deleted.

83 changes: 46 additions & 37 deletions packages/gatsby/src/joi-schemas/__tests__/joi.ts
Original file line number Diff line number Diff line change
@@ -1,109 +1,116 @@
import { gatsbyConfigSchema, nodeSchema } from "../joi"

describe(`gatsby config`, () => {
it(`returns empty pathPrefix when not set`, async () => {
it(`returns empty pathPrefix when not set`, () => {
const config = {}

const result = await gatsbyConfigSchema.validate(config)
expect(result).toEqual(
const result = gatsbyConfigSchema.validate(config)
expect(result.value).toEqual(
expect.objectContaining({
pathPrefix: ``,
})
)
})

it(`strips trailing slashes from url fields`, async () => {
it(`throws when linkPrefix is set`, () => {
const config = {
linkPrefix: `/blog/`,
}

const result = gatsbyConfigSchema.validate(config)
expect(result.error).toMatchInlineSnapshot(
`[Error: "linkPrefix" should be changed to "pathPrefix"]`
)
})

it(`strips trailing slashes from url fields`, () => {
const config = {
pathPrefix: `/blog///`,
assetPrefix: `https://cdn.example.com/`,
}

const result = await gatsbyConfigSchema.validate(config)
expect(result).toEqual(
const result = gatsbyConfigSchema.validate(config)
expect(result.value).toEqual(
expect.objectContaining({
pathPrefix: `/blog`,
assetPrefix: `https://cdn.example.com`,
})
)
})

it(`allows assetPrefix to be full URL`, async () => {
it(`allows assetPrefix to be full URL`, () => {
const config = {
assetPrefix: `https://cdn.example.com/`,
}

const result = await gatsbyConfigSchema.validate(config)
expect(result).toEqual(
const result = gatsbyConfigSchema.validate(config)
expect(result.value).toEqual(
expect.objectContaining({
assetPrefix: `https://cdn.example.com`,
})
)
})

it(`allows assetPrefix to be a URL with nested paths`, async () => {
it(`allows assetPrefix to be a URL with nested paths`, () => {
const config = {
assetPrefix: `https://cdn.example.com/some/nested/path`,
}

const result = await gatsbyConfigSchema.validate(config)
expect(result).toEqual(expect.objectContaining(config))
const result = gatsbyConfigSchema.validate(config)
expect(result.value).toEqual(expect.objectContaining(config))
})

it(`allows relative paths for url fields`, async () => {
it(`allows relative paths for url fields`, () => {
const config = {
pathPrefix: `/blog`,
assetPrefix: `https://cdn.example.com`,
}

const result = await gatsbyConfigSchema.validate(config)
expect(result).toEqual(expect.objectContaining(config))
const result = gatsbyConfigSchema.validate(config)
expect(result.value).toEqual(expect.objectContaining(config))
})

it(`strips trailing slash and add leading slash to pathPrefix`, async () => {
it(`strips trailing slash and add leading slash to pathPrefix`, () => {
const config = {
pathPrefix: `blog/`,
assetPrefix: `https://cdn.example.com/`,
}

const result = await gatsbyConfigSchema.validate(config)
expect(result).toEqual(
const result = gatsbyConfigSchema.validate(config)
expect(result.value).toEqual(
expect.objectContaining({
pathPrefix: `/blog`,
assetPrefix: `https://cdn.example.com`,
})
)
})

it(`does not allow pathPrefix to be full URL`, async () => {
expect.assertions(1)
it(`does not allow pathPrefix to be full URL`, () => {
const config = {
pathPrefix: `https://google.com`,
}

try {
await gatsbyConfigSchema.validate(config)
} catch (err) {
expect(err.message).toMatchSnapshot()
}
const result = gatsbyConfigSchema.validate(config)
expect(result.error).toMatchInlineSnapshot(
`[ValidationError: "pathPrefix" must be a valid relative uri]`
)
})

it(`throws when relative path used for both assetPrefix and pathPrefix`, async () => {
expect.assertions(1)
it(`throws when relative path used for both assetPrefix and pathPrefix`, () => {
const config = {
assetPrefix: `/assets`,
pathPrefix: `/blog`,
}

try {
await gatsbyConfigSchema.validate(config)
} catch (err) {
expect(err.message).toMatchSnapshot()
}
const result = gatsbyConfigSchema.validate(config)
expect(result.error).toMatchInlineSnapshot(
`[Error: assetPrefix must be an absolute URI when used with pathPrefix]`
)
})
})

describe(`node schema`, () => {
it(`allows correct nodes`, async () => {
it(`allows correct nodes`, () => {
const node = {
id: `foo`,
internal: {
Expand All @@ -119,7 +126,7 @@ describe(`node schema`, () => {
expect(error).toBeFalsy()
})

it(`force id type`, async () => {
it(`force id type`, () => {
const node = {
id: 5,
internal: {
Expand All @@ -134,10 +141,10 @@ describe(`node schema`, () => {
const { error } = nodeSchema.validate(node)
expect(error).toBeTruthy()
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
expect(error!.message).toMatchSnapshot()
expect(error!.message).toMatchInlineSnapshot(`"\\"id\\" must be a string"`)
})

it(`doesn't allow unknown internal fields`, async () => {
it(`doesn't allow unknown internal fields`, () => {
const node = {
id: `foo`,
internal: {
Expand All @@ -154,6 +161,8 @@ describe(`node schema`, () => {
const { error } = nodeSchema.validate(node)
expect(error).toBeTruthy()
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
expect(error!.message).toMatchSnapshot()
expect(error!.message).toMatchInlineSnapshot(
`"\\"internal.customField\\" is not allowed"`
)
})
})
2 changes: 1 addition & 1 deletion packages/gatsby/src/joi-schemas/joi.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Joi from "@hapi/joi"
import Joi from "joi"
import { IGatsbyConfig, IGatsbyPage, IGatsbyNode } from "../redux/types"

const stripTrailingSlash = (chain: Joi.StringSchema): Joi.StringSchema =>
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby/src/redux/actions/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ export const setSiteConfig = (config?: unknown): ISetSiteConfig => {

if (result.error) {
const hasUnknownKeys = result.error.details.filter(
details => details.type === `object.allowUnknown`
details => details.type === `object.unknown`
)

if (Array.isArray(hasUnknownKeys) && hasUnknownKeys.length) {
Expand Down
3 changes: 1 addition & 2 deletions packages/gatsby/src/redux/actions/public.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// @flow
const Joi = require(`@hapi/joi`)
const chalk = require(`chalk`)
const _ = require(`lodash`)
const { stripIndent } = require(`common-tags`)
Expand Down Expand Up @@ -647,7 +646,7 @@ const createNode = (

trackCli(`CREATE_NODE`, trackParams, { debounce: true })

const result = Joi.validate(node, nodeSchema)
const result = nodeSchema.validate(node)
if (result.error) {
if (!hasErroredBecauseOfNodeValidation.has(result.error.message)) {
const errorObj = {
Expand Down
Loading

0 comments on commit 86b8b26

Please sign in to comment.