-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
fix(gatsby): log config validation errors #23620
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import { setSiteConfig } from "../internal" | ||
import reporter from "gatsby-cli/lib/reporter" | ||
|
||
jest.mock(`gatsby-cli/lib/reporter`, () => { | ||
return { | ||
panic: jest.fn(), | ||
} | ||
}) | ||
|
||
beforeEach(() => { | ||
reporter.panic.mockClear() | ||
}) | ||
|
||
describe(`setSiteConfig`, () => { | ||
it(`let's you add a config`, () => { | ||
const action = setSiteConfig({ | ||
siteMetadata: { | ||
hi: true, | ||
}, | ||
}) | ||
expect(action).toMatchInlineSnapshot(` | ||
Object { | ||
"payload": Object { | ||
"pathPrefix": "", | ||
"polyfill": true, | ||
"siteMetadata": Object { | ||
"hi": true, | ||
}, | ||
}, | ||
"type": "SET_SITE_CONFIG", | ||
} | ||
`) | ||
}) | ||
|
||
it(`handles empty configs`, () => { | ||
const action = setSiteConfig() | ||
expect(action).toMatchInlineSnapshot(` | ||
Object { | ||
"payload": Object { | ||
"pathPrefix": "", | ||
"polyfill": true, | ||
}, | ||
"type": "SET_SITE_CONFIG", | ||
} | ||
`) | ||
}) | ||
|
||
it(`Validates configs with unsupported options`, () => { | ||
setSiteConfig({ | ||
someRandomThing: `hi people`, | ||
plugins: [], | ||
}) | ||
|
||
expect(reporter.panic).toBeCalledWith({ | ||
id: `10122`, | ||
context: { | ||
sourceMessage: `"someRandomThing" is not allowed`, | ||
}, | ||
}) | ||
}) | ||
|
||
it(`It corrects pathPrefixes without a forward slash at beginning`, () => { | ||
const action = setSiteConfig({ | ||
pathPrefix: `prefix`, | ||
}) | ||
|
||
expect(action.payload.pathPrefix).toBe(`/prefix`) | ||
}) | ||
|
||
it(`It removes trailing forward slash`, () => { | ||
const action = setSiteConfig({ | ||
pathPrefix: `/prefix/`, | ||
}) | ||
expect(action.payload.pathPrefix).toBe(`/prefix`) | ||
}) | ||
|
||
it(`It removes pathPrefixes that are a single forward slash`, () => { | ||
const action = setSiteConfig({ | ||
pathPrefix: `/`, | ||
}) | ||
expect(action.payload.pathPrefix).toBe(``) | ||
}) | ||
|
||
it(`It sets the pathPrefix to an empty string if it's not set`, () => { | ||
const action = setSiteConfig({}) | ||
expect(action.payload.pathPrefix).toBe(``) | ||
}) | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Those are tests that previously were reducer tests adjusted to be action creator test (same scenarios) |
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { IGatsbyConfig, ISetSiteConfig } from "../types" | ||
|
||
module.exports = ( | ||
state: IGatsbyConfig = {}, | ||
action: ISetSiteConfig | ||
): IGatsbyConfig => { | ||
switch (action.type) { | ||
case `SET_SITE_CONFIG`: { | ||
return action.payload | ||
} | ||
default: | ||
return state | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,9 @@ export interface IGatsbyConfig { | |
title?: string | ||
author?: string | ||
description?: string | ||
sireUrl?: string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo here... --> #24488 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙈 Thanks! |
||
// siteMetadata is free form | ||
[key: string]: unknown | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fail There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean - in context of gatsby core it doesn't matter much, because core doesn't make use of those fields really There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But it might matter when we use those types for public There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, doesn't matter at all Just weird to see those There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I needed it here I think for tests in particular which were setting random thing (really random):
---edit - that was wrong example - see comment below There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. above was wrong one - that one was failing - it was for this one:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is almost as good as |
||
} | ||
// @deprecated | ||
polyfill?: boolean | ||
|
@@ -422,3 +425,8 @@ export interface ISetWebpackConfigAction { | |
type: `SET_WEBPACK_CONFIG` | ||
payload: Partial<IGatsbyState["webpack"]> | ||
} | ||
|
||
export interface ISetSiteConfig { | ||
type: `SET_SITE_CONFIG` | ||
payload: IGatsbyState["config"] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved from
gatsby/packages/gatsby/src/redux/reducers/config.js
Lines 21 to 23 in 874088c