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

fix(gatsby-plugin-page-creator): juggle reporter dont depend on cli #26357

Merged
merged 2 commits into from
Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe(`isValidCollectionPathImplementation`, () => {
`%o passes`,
path => {
expect(() =>
isValidCollectionPathImplementation(compatiblePath(path))
isValidCollectionPathImplementation(compatiblePath(path), reporter)
).not.toThrow()
}
)
Expand All @@ -29,7 +29,7 @@ describe(`isValidCollectionPathImplementation`, () => {
])(`%o throws as expected`, path => {
const part = path.split(`/`)[2]

isValidCollectionPathImplementation(compatiblePath(path))
isValidCollectionPathImplementation(compatiblePath(path), reporter)
expect(reporter.panicOnBuild)
.toBeCalledWith(`Collection page builder encountered an error parsing the filepath. To use collection paths the schema to follow is {Model.field}. The problematic part is: ${part}.
filePath: ${compatiblePath(path)}`)
Expand Down
12 changes: 10 additions & 2 deletions packages/gatsby-plugin-page-creator/src/create-page-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { createClientOnlyPage } from "./create-client-only-page"
import { createPagesFromCollectionBuilder } from "./create-pages-from-collection-builder"
import systemPath from "path"
import { trackFeatureIsUsed } from "gatsby-telemetry"
import { Reporter } from "gatsby"
pvdz marked this conversation as resolved.
Show resolved Hide resolved

function pathIsCollectionBuilder(path: string): boolean {
return path.includes(`{`)
Expand All @@ -18,7 +19,8 @@ export function createPage(
pagesDirectory: string,
actions: Actions,
ignore: string[],
graphql: CreatePagesArgs["graphql"]
graphql: CreatePagesArgs["graphql"],
reporter: Reporter
): void {
// Filter out special components that shouldn't be made into
// pages.
Expand All @@ -41,7 +43,13 @@ export function createPage(
`PageCreator: Found a collection route, but the proper env was not set to enable this experimental feature. Please run again with \`GATSBY_EXPERIMENTAL_ROUTING_APIS=1\` to enable.`
)
}
createPagesFromCollectionBuilder(filePath, absolutePath, actions, graphql)
createPagesFromCollectionBuilder(
filePath,
absolutePath,
actions,
graphql,
reporter
)
return
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,24 @@ import { derivePath } from "./derive-path"
import { watchCollectionBuilder } from "./watch-collection-builder"
import { collectionExtractQueryString } from "./collection-extract-query-string"
import { isValidCollectionPathImplementation } from "./is-valid-collection-path-implementation"
import reporter from "gatsby-cli/lib/reporter"

// TODO: Do we need the ignore argument?
export async function createPagesFromCollectionBuilder(
filePath: string,
absolutePath: string,
actions: Actions,
graphql: CreatePagesArgs["graphql"]
graphql: CreatePagesArgs["graphql"],
reporter
): Promise<void> {
if (isValidCollectionPathImplementation(absolutePath) === false) {
if (isValidCollectionPathImplementation(absolutePath, reporter) === false) {
watchCollectionBuilder(absolutePath, ``, [], actions, () =>
createPagesFromCollectionBuilder(filePath, absolutePath, actions, graphql)
createPagesFromCollectionBuilder(
filePath,
absolutePath,
actions,
graphql,
reporter
)
)
return
}
Expand All @@ -30,7 +36,13 @@ export async function createPagesFromCollectionBuilder(
// 1.a If the query string is not findable, we can't move on. So we stop and watch
if (queryString === null) {
watchCollectionBuilder(absolutePath, ``, [], actions, () =>
createPagesFromCollectionBuilder(filePath, absolutePath, actions, graphql)
createPagesFromCollectionBuilder(
filePath,
absolutePath,
actions,
graphql,
reporter
)
)
return
}
Expand All @@ -51,7 +63,13 @@ ${errors.map(error => error.message).join(`\n`)}`.trim()
)

watchCollectionBuilder(absolutePath, queryString, [], actions, () =>
createPagesFromCollectionBuilder(filePath, absolutePath, actions, graphql)
createPagesFromCollectionBuilder(
filePath,
absolutePath,
actions,
graphql,
reporter
)
)

return
Expand Down Expand Up @@ -97,6 +115,12 @@ ${errors.map(error => error.message).join(`\n`)}`.trim()
})

watchCollectionBuilder(absolutePath, queryString, paths, actions, () =>
createPagesFromCollectionBuilder(filePath, absolutePath, actions, graphql)
createPagesFromCollectionBuilder(
filePath,
absolutePath,
actions,
graphql,
reporter
)
)
}
11 changes: 9 additions & 2 deletions packages/gatsby-plugin-page-creator/src/gatsby-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,22 @@ Please pick a path to an existing directory.`)
// Get initial list of files.
let files = await glob(pagesGlob, { cwd: pagesPath })
files.forEach(file => {
createPage(file, pagesDirectory, actions, ignore, graphql)
createPage(file, pagesDirectory, actions, ignore, graphql, reporter)
})

watchDirectory(
pagesPath,
pagesGlob,
addedPath => {
if (!_.includes(files, addedPath)) {
createPage(addedPath, pagesDirectory, actions, ignore, graphql)
createPage(
addedPath,
pagesDirectory,
actions,
ignore,
graphql,
reporter
)
files.push(addedPath)
}
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import sysPath from "path"
import reporter from "gatsby-cli/lib/reporter"
import { Reporter } from "gatsby"
pvdz marked this conversation as resolved.
Show resolved Hide resolved

// This file is a helper for consumers. It's going to log an error to them if they
// in any way have an incorrect filepath setup for us to predictably use collection
// querying.
//
// Without this, users will can get mystic errors.
export function isValidCollectionPathImplementation(filePath: string): boolean {
export function isValidCollectionPathImplementation(
filePath: string,
reporter: Reporter
): boolean {
const parts = filePath.split(sysPath.sep)
let passing = true

Expand Down