Skip to content

Commit 68fe2ef

Browse files
mhd-slnKyleAMathews
authored andcommitted
Automatic file validation for components in src/pages (#3881)
* yarn.lock * add page validation and fix breaking createPage action tests * Improve error messages for invalid pages
1 parent da572ca commit 68fe2ef

File tree

5 files changed

+610
-686
lines changed

5 files changed

+610
-686
lines changed

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
"eslint-plugin-react": "^7.3.0",
2828
"flow-bin": "^0.42.0",
2929
"glob": "^7.1.1",
30-
"jest": "^20.0.4",
31-
"jest-cli": "^20.0.4",
30+
"jest": "^22.1.4",
31+
"jest-cli": "^22.1.4",
3232
"lerna": "^2.1.1",
3333
"npm-run-all": "4.1.2",
3434
"plop": "^1.8.1",

packages/gatsby/__mocks__/fs.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
'use strict'
2+
3+
const fs = jest.genMockFromModule(`fs`)
4+
5+
let mockFiles = {}
6+
function __setMockFiles(newMockFiles) {
7+
mockFiles = Object.assign({}, newMockFiles)
8+
}
9+
10+
function readFileSync(filePath, parser) {
11+
return mockFiles[filePath]
12+
}
13+
14+
fs.__setMockFiles = __setMockFiles
15+
fs.readFileSync = readFileSync
16+
17+
module.exports = fs

packages/gatsby/src/redux/__tests__/pages.js

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,29 @@
1+
'use strict'
2+
3+
const glob = require(`glob`)
14
const reducer = require(`../reducers/pages`)
2-
const { actions } = require(`../actions`)
5+
6+
jest.mock(`fs`)
37

48
Date.now = jest.fn(
59
() =>
610
// const diff = new Date().getTime() - start
711
1482363367071 // + diff
812
)
913

14+
glob.sync = jest.fn(() => ``)
15+
1016
describe(`Add pages`, () => {
11-
it(`allows you to add pages`, () => {
17+
const MOCK_FILE_INFO = {
18+
'/whatever/index.js': `import React from 'react'; export default Page;`,
19+
'/whatever2/index.js': `import React from 'react'; export default Page;`,
20+
}
21+
beforeEach(() => {
22+
// Set up some mocked out file info before each test
23+
require(`fs`).__setMockFiles(MOCK_FILE_INFO)
24+
})
25+
test(`allows you to add pages`, () => {
26+
const { actions } = require(`../actions`)
1227
const action = actions.createPage(
1328
{
1429
path: `/hi/`,
@@ -22,16 +37,18 @@ describe(`Add pages`, () => {
2237
})
2338

2439
it(`Fails if path is missing`, () => {
40+
const { actions } = require(`../actions`)
2541
const action = actions.createPage(
2642
{
27-
component: `/whatever/index.js`,
43+
component: `/path/to/file1.js`,
2844
},
2945
{ id: `test`, name: `test` }
3046
)
3147
expect(action).toMatchSnapshot()
3248
})
3349

3450
it(`Fails if component path is missing`, () => {
51+
const { actions } = require(`../actions`)
3552
const action = actions.createPage(
3653
{
3754
path: `/whatever/`,
@@ -42,6 +59,7 @@ describe(`Add pages`, () => {
4259
})
4360

4461
it(`Fails if the component path isn't absolute`, () => {
62+
const { actions } = require(`../actions`)
4563
const action = actions.createPage(
4664
{
4765
path: `/whatever/`,
@@ -53,6 +71,7 @@ describe(`Add pages`, () => {
5371
})
5472

5573
it(`adds an initial forward slash if the user doesn't`, () => {
74+
const { actions } = require(`../actions`)
5675
const action = actions.createPage(
5776
{
5877
path: `hi/`,
@@ -65,6 +84,7 @@ describe(`Add pages`, () => {
6584
})
6685

6786
it(`allows you to add pages with context`, () => {
87+
const { actions } = require(`../actions`)
6888
const action = actions.createPage(
6989
{
7090
path: `/hi/`,
@@ -81,6 +101,7 @@ describe(`Add pages`, () => {
81101
})
82102

83103
it(`allows you to add pages with matchPath`, () => {
104+
const { actions } = require(`../actions`)
84105
const action = actions.createPage(
85106
{
86107
path: `/hi/`,
@@ -95,6 +116,7 @@ describe(`Add pages`, () => {
95116
})
96117

97118
it(`allows you to add multiple pages`, () => {
119+
const { actions } = require(`../actions`)
98120
const action = actions.createPage(
99121
{
100122
path: `/hi/`,
@@ -116,6 +138,7 @@ describe(`Add pages`, () => {
116138
})
117139

118140
it(`allows you to update existing pages (based on path)`, () => {
141+
const { actions } = require(`../actions`)
119142
const action = actions.createPage(
120143
{
121144
path: `/hi/`,
@@ -140,6 +163,7 @@ describe(`Add pages`, () => {
140163
})
141164

142165
it(`allows you to delete paths`, () => {
166+
const { actions } = require(`../actions`)
143167
const action = actions.createPage(
144168
{
145169
path: `/hi/`,

packages/gatsby/src/redux/actions.js

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const { bindActionCreators } = require(`redux`)
66
const { stripIndent } = require(`common-tags`)
77
const glob = require(`glob`)
88
const path = require(`path`)
9-
9+
const fs = require(`fs`)
1010
const { joinPath } = require(`../utils/path`)
1111
const {
1212
getNode,
@@ -187,6 +187,72 @@ actions.createPage = (page: PageInput, plugin?: Plugin, traceId?: string) => {
187187
return null
188188
}
189189

190+
// Validate that the page component imports React and exports something
191+
// (hopefully a component).
192+
if (!internalPage.component.includes(`/.cache/`)) {
193+
const fileContent = fs.readFileSync(internalPage.component, `utf-8`)
194+
let notEmpty = true
195+
let includesReactImport = true
196+
let includesDefaultExport = true
197+
198+
if (fileContent === ``) {
199+
notEmpty = false
200+
}
201+
202+
if (!fileContent.includes(`React`)) {
203+
includesReactImport = false
204+
}
205+
if (
206+
!fileContent.includes(`export default`) &&
207+
!fileContent.includes(`module.exports`)
208+
) {
209+
includesDefaultExport = false
210+
}
211+
if (!notEmpty || !includesDefaultExport || !includesReactImport) {
212+
const relativePath = path.relative(
213+
store.getState().program.directory,
214+
internalPage.component
215+
)
216+
217+
if (!notEmpty) {
218+
console.log(``)
219+
console.log(
220+
`You have an empty file in the "src/pages" directory at "${relativePath}". Please remove it or make it a valid component`
221+
)
222+
console.log(``)
223+
process.exit(1)
224+
}
225+
226+
console.log(``)
227+
console.log(``)
228+
console.log(
229+
`The page component at "${relativePath}" didn't pass validation`
230+
)
231+
232+
if (!includesReactImport) {
233+
console.log(``)
234+
console.log(
235+
`You must import React at the top of the file for a React component to be valid`
236+
)
237+
console.log(``)
238+
console.log(`Add the following to the top of the component:`)
239+
console.log(``)
240+
console.log(` import React from 'react'`)
241+
console.log(``)
242+
}
243+
244+
if (!includesDefaultExport) {
245+
console.log(``)
246+
console.log(
247+
`The page component must export a React component for it to be valid`
248+
)
249+
console.log(``)
250+
}
251+
252+
process.exit(1)
253+
}
254+
}
255+
190256
return {
191257
type: `CREATE_PAGE`,
192258
plugin,

0 commit comments

Comments
 (0)