From 66bf51c061ca4e89e2db0cbbfeebb85181fa883d Mon Sep 17 00:00:00 2001 From: Benjamin Stepp Date: Sun, 3 Apr 2016 16:40:13 -0500 Subject: [PATCH 1/3] Extract the resolving of a page's path and test it * Extracts the resolving of a page's path to it's own file for testability. * Adds `ava` as a development dependency to write and run tests. * Adds ava related settings to inherit the babel config and run the tests with npm test to package.json * Refactor the quad-nested if statements in path resolving. Working toward resolving Issue #154 --- lib/utils/glob-pages.js | 41 ++------------------------ lib/utils/path-resolver.js | 53 +++++++++++++++++++++++++++++++++ package.json | 7 ++++- test/utils/path-resolver.js | 58 +++++++++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 39 deletions(-) create mode 100644 lib/utils/path-resolver.js create mode 100644 test/utils/path-resolver.js diff --git a/lib/utils/glob-pages.js b/lib/utils/glob-pages.js index 5588ab347abcb..6641a29399b79 100644 --- a/lib/utils/glob-pages.js +++ b/lib/utils/glob-pages.js @@ -6,15 +6,8 @@ import fs from 'fs' import frontMatter from 'front-matter' import htmlFrontMatter from 'html-frontmatter' import objectAssign from 'object-assign' +import pathResolver from './path-resolver' const debug = require('debug')('gatsby:glob') -let rewritePath -try { - const gatsbyNodeConfig = path.resolve(process.cwd(), './gatsby-node.js') - const nodeConfig = require(gatsbyNodeConfig) - rewritePath = nodeConfig.rewritePath -} catch (e) { - // Ignore -} module.exports = (directory, callback) => { const pagesData = [] @@ -71,38 +64,10 @@ module.exports = (directory, callback) => { data = {} } - // Determine path for page (unless it's a file that starts with an - // underscore as these don't become pages). - if (!(parsed.name.slice(0, 1) === '_')) { - if (data.path) { - // Path was hardcoded. - pageData.path = data.path - } else if (rewritePath) { - pageData.path = rewritePath(parsed, pageData) - } - - // If the above didn't set a path, set our own. - if (!pageData.path) { - // If this is an index page or template, it's path is /foo/bar/ - if (parsed.name === 'index' || parsed.name === 'template') { - if (parsed.dirname === '') { - pageData.path = '/' - } else { - pageData.path = `/${parsed.dirname}/` - } - // Else if not an index, create a path like /foo/bar/ - // and rely upon static-site-generator-webpack-plugin to add index.html - } else { - if (parsed.dirname === '') { - pageData.path = `/${parsed.name}/` - } else { - pageData.path = `/${parsed.dirname}/${parsed.name}/` - } - } - } + pageData.path = pathResolver(data, parsed) // Set the "template path" - } else if (parsed.name === '_template') { + if (parsed.name === '_template') { pageData.templatePath = `/${parsed.dirname}/` } diff --git a/lib/utils/path-resolver.js b/lib/utils/path-resolver.js new file mode 100644 index 0000000000000..f33d6154ec645 --- /dev/null +++ b/lib/utils/path-resolver.js @@ -0,0 +1,53 @@ +import { posix as path } from 'path' +import { startsWith } from 'lodash' + +let rewritePath +try { + const gatsbyNodeConfig = path.resolve(process.cwd(), './gatsby-node.js') + const nodeConfig = require(gatsbyNodeConfig) + rewritePath = nodeConfig.rewritePath +} catch (e) { + // Ignore +} + +export default function pathResolver (pageData, parsedPath) { + /** + * Determines if a hardcoded path was given in the frontmatter of a page. + */ + function hardcodedPath () { + return pageData.path + } + + /** + * Determines if the path should be rewritten using rules provided by the + * user in the gatsby-node.js config file in the root of the project. + */ + function rewrittenPath () { + if (rewritePath) { + return rewritePath(parsedPath, pageData) + } else { return undefined } + } + + /** + * Determines the path of the page using the default of its location on the + * filesystem. + */ + function defaultPath () { + const { dirname } = parsedPath + let { name } = parsedPath + if (name === 'template' || name === 'index') { + name = '' + } + return path.join('/', dirname, name, '/') + } + + /** + * Returns a path for a page. If the page name starts with an underscore, + * undefined is returned as it does not become a page. + */ + if (!startsWith(parsedPath.name, '_')) { + return hardcodedPath() || rewrittenPath() || defaultPath() + } else { + return undefined + } +} diff --git a/package.json b/package.json index c0ebfa670e76e..f4a69717dcc5f 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "yaml-loader": "^0.1.0" }, "devDependencies": { + "ava": "^0.13.0", "babel-cli": "^6.6.5", "babel-eslint": "^6.0.0", "eslint": "^2.4.0", @@ -109,7 +110,11 @@ "scripts": { "build": "babel lib --out-dir dist/", "lint": "eslint --ext .js,.jsx --ignore-pattern dist .", - "test": "npm run lint", + "test": "node_modules/.bin/ava", "watch": "babel -w lib --out-dir dist/" + }, + "ava": { + "require": [ "babel-register" ], + "babel": "inherit" } } diff --git a/test/utils/path-resolver.js b/test/utils/path-resolver.js new file mode 100644 index 0000000000000..bc9a1f34c6b38 --- /dev/null +++ b/test/utils/path-resolver.js @@ -0,0 +1,58 @@ +import test from 'ava' +import pathResolver from '../../lib/utils/path-resolver' + +test('returns undefined when filename starts with _', t => { + const parsedPath = { name: '_notapage' } + const data = {} + const pagePath = pathResolver(data, parsedPath) + t.true(pagePath === undefined) +}) + +test('uses the path from page data', t => { + const parsedPath = { name: 'page', dirname: 'a-directory/' } + const data = { path: '/page-at-root' } + const pagePath = pathResolver(data, parsedPath) + t.same(pagePath, '/page-at-root') +}) + +test('removes index from path', t => { + const parsedPath = { name: 'index', dirname: 'foo/bar' } + const data = {} + const pagePath = pathResolver(data, parsedPath) + t.same(pagePath, '/foo/bar/') +}) + +test('removes template from path', t => { + const parsedPath = { name: 'template', dirname: 'foo/bar' } + const data = {} + const pagePath = pathResolver(data, parsedPath) + t.same(pagePath, '/foo/bar/') +}) + +test('template at root has root path', t => { + const parsedPath = { name: 'template', dirname: '' } + const data = {} + const pagePath = pathResolver(data, parsedPath) + t.same(pagePath, '/') +}) + +test('index at root has root path', t => { + const parsedPath = { name: 'index', dirname: '' } + const data = {} + const pagePath = pathResolver(data, parsedPath) + t.same(pagePath, '/') +}) + +test('if not an index create a path like /foo/bar/', t => { + const parsedPath = { name: 'creating-static-websites', dirname: 'blog/2016' } + const data = {} + const pagePath = pathResolver(data, parsedPath) + t.same(pagePath, '/blog/2016/creating-static-websites/') +}) + +test('if not an index create a path like /foo/bar/', t => { + const parsedPath = { name: 'about-us', dirname: '' } + const data = {} + const pagePath = pathResolver(data, parsedPath) + t.same(pagePath, '/about-us/') +}) From cb5d14030345fd1119d10befc5260b8042f2bbdb Mon Sep 17 00:00:00 2001 From: Benjamin Stepp Date: Mon, 4 Apr 2016 23:10:52 -0500 Subject: [PATCH 2/3] Separate building Page Data and Globbing Pages * Split up all the spaghetti in glob-pages up so that it can be unit tested. * Renamed `pathresolver` to `urlResolver` * Added tests to verify the structure of the data returned by the new pathResolver. * updated the urlResolver's tests with the new function name --- lib/utils/build-page/index.js | 13 +++++ lib/utils/build-page/load-frontmatter.js | 27 +++++++++ lib/utils/build-page/path-resolver.js | 28 ++++++++++ .../url-resolver.js} | 0 lib/utils/glob-pages.js | 50 +---------------- test/utils/build-page/path-resolver.js | 56 +++++++++++++++++++ .../url-resolver.js} | 18 +++--- 7 files changed, 135 insertions(+), 57 deletions(-) create mode 100644 lib/utils/build-page/index.js create mode 100644 lib/utils/build-page/load-frontmatter.js create mode 100644 lib/utils/build-page/path-resolver.js rename lib/utils/{path-resolver.js => build-page/url-resolver.js} (100%) create mode 100644 test/utils/build-page/path-resolver.js rename test/utils/{path-resolver.js => build-page/url-resolver.js} (74%) diff --git a/lib/utils/build-page/index.js b/lib/utils/build-page/index.js new file mode 100644 index 0000000000000..55afcef971208 --- /dev/null +++ b/lib/utils/build-page/index.js @@ -0,0 +1,13 @@ +import path from 'path' +import objectAssign from 'object-assign' +import pathResolver from './path-resolver' +import loadFrontmatter from './load-frontmatter' + +export default function buildPage (directory, page) { + const pageData = loadFrontmatter(page) + + const relativePath = path.relative(path.join(directory, 'pages'), page) + const pathData = pathResolver(relativePath) + + return objectAssign({}, pageData, pathData) +} diff --git a/lib/utils/build-page/load-frontmatter.js b/lib/utils/build-page/load-frontmatter.js new file mode 100644 index 0000000000000..d1148873b080b --- /dev/null +++ b/lib/utils/build-page/load-frontmatter.js @@ -0,0 +1,27 @@ +import fs from 'fs' +import path from 'path' +import frontMatter from 'front-matter' +import objectAssign from 'object-assign' +import htmlFrontMatter from 'html-frontmatter' + +export default function loadFrontmatter (pagePath) { + const ext = path.extname(pagePath).slice(1) + + // Load data for each file type. + // TODO use webpack-require to ensure data loaded + // here (in node context) is consistent with what's loaded + // in the browser. + + let data + if (ext === 'md') { + const rawData = frontMatter(fs.readFileSync(pagePath, 'utf-8')) + data = objectAssign({}, rawData.attributes) + } else if (ext === 'html') { + const html = fs.readFileSync(pagePath, 'utf-8') + data = objectAssign({}, htmlFrontMatter(html), { body: html }) + } else { + data = {} + } + + return data +} diff --git a/lib/utils/build-page/path-resolver.js b/lib/utils/build-page/path-resolver.js new file mode 100644 index 0000000000000..32aacd00ec219 --- /dev/null +++ b/lib/utils/build-page/path-resolver.js @@ -0,0 +1,28 @@ +import slash from 'slash' +import parsePath from 'parse-filepath' +import urlResolver from './url-resolver' + +export default function pathResolver (relativePath) { + const data = {} + + data.file = parsePath(relativePath) + + // Remove the . from extname (.md -> md) + data.file.ext = data.file.extname.slice(1) + // Make sure slashes on parsed.dirname are correct for Windows + data.file.dirname = slash(data.file.dirname) + + // Determine require path + data.requirePath = slash(relativePath) + + // set the URL path (should this be renamed) + // and now looking at it, it only needs a reference to pageData + data.path = urlResolver(data, data.file) + + // Set the "template path" + if (data.file.name === '_template') { + data.templatePath = `/${data.file.dirname}/` + } + + return data +} diff --git a/lib/utils/path-resolver.js b/lib/utils/build-page/url-resolver.js similarity index 100% rename from lib/utils/path-resolver.js rename to lib/utils/build-page/url-resolver.js diff --git a/lib/utils/glob-pages.js b/lib/utils/glob-pages.js index 6641a29399b79..c9f98d1a1cb94 100644 --- a/lib/utils/glob-pages.js +++ b/lib/utils/glob-pages.js @@ -1,12 +1,5 @@ import glob from 'glob' -import path from 'path' -import parsePath from 'parse-filepath' -import slash from 'slash' -import fs from 'fs' -import frontMatter from 'front-matter' -import htmlFrontMatter from 'html-frontmatter' -import objectAssign from 'object-assign' -import pathResolver from './path-resolver' +import buildPage from './build-page' const debug = require('debug')('gatsby:glob') module.exports = (directory, callback) => { @@ -32,46 +25,7 @@ module.exports = (directory, callback) => { if (err) { return callback(err) } pages.forEach((page) => { - const pageData = {} - pageData.file = {} - - pageData.file = parsePath(path.relative(`${directory}/pages`, page)) - const parsed = pageData.file - - pageData.file.ext = parsed.extname.slice(1) - const ext = pageData.file.ext - - // Determine require path - pageData.requirePath = slash(path.relative(`${directory}/pages`, page)) - - // Make sure slashes on parsed.dirname are correct for Windows - parsed.dirname = slash(parsed.dirname) - - // Load data for each file type. - // TODO use webpack-require to ensure data loaded - // here (in node context) is consistent with what's loaded - // in the browser. - let data - if (ext === 'md') { - const rawData = frontMatter(fs.readFileSync(page, 'utf-8')) - data = objectAssign({}, rawData.attributes) - pageData.data = data - } else if (ext === 'html') { - const html = fs.readFileSync(page, 'utf-8') - data = objectAssign({}, htmlFrontMatter(html), { body: html }) - pageData.data = data - } else { - data = {} - } - - pageData.path = pathResolver(data, parsed) - - // Set the "template path" - if (parsed.name === '_template') { - pageData.templatePath = `/${parsed.dirname}/` - } - - pagesData.push(pageData) + pagesData.push(buildPage(directory, page)) }) debug(`globbed ${pagesData.length} pages`) diff --git a/test/utils/build-page/path-resolver.js b/test/utils/build-page/path-resolver.js new file mode 100644 index 0000000000000..28d0a4481428d --- /dev/null +++ b/test/utils/build-page/path-resolver.js @@ -0,0 +1,56 @@ +import test from 'ava' +import path from 'path' +import { startsWith } from 'lodash' +import pathResolver from '../../../lib/utils/build-page/path-resolver' + +test('it returns an object', t => { + const pagePath = '/' + const result = pathResolver(pagePath) + t.ok(result) + t.is(typeof result, 'object') +}) + +test('it has a require path', t => { + const pagePath = '/a-blog-post/index.md' + const result = pathResolver(pagePath) + t.ok(result.requirePath) + t.is(typeof result.requirePath, 'string') +}) + +test('it does not has a path if the name start with _', t => { + t.is(pathResolver('/_metadata.js').path, undefined) + t.is(pathResolver('/_template.html.erb').path, undefined) + t.is(pathResolver('/_notapage.json').path, undefined) +}) + +test('it has a path if the name doesnt start with _', t => { + t.ok(pathResolver('/2015/back-to-the-future.md').path) + t.ok(pathResolver('/my-first-blog.md').path) + t.ok(pathResolver('/index.md').path) +}) + +test('it has a templatePath if the name is _template', t => { + const pagePath = '/_template.js' + const result = pathResolver(pagePath) + t.ok(result.templatePath) + t.is(typeof result.templatePath, 'string') +}) + +test('it does not have a templatePath if not a _template', t => { + t.is(pathResolver('/index.md').templatePath, undefined) + t.is(pathResolver('/another-page.toml').templatePath, undefined) + t.is(pathResolver('/something-else/good-stuff.html').templatePath, undefined) +}) + +test('the directory name has / slashes', t => { + const pagePath = '/2016/testing-middleman-sites-with-capybara/index.md' + const result = pathResolver(pagePath) + + t.same(result.file.dirname, path.posix.normalize(result.file.dirname)) + t.same(result.file.dirname, '/2016/testing-middleman-sites-with-capybara') +}) + +test('the ext doesnt have a leading .', t => { + const result = pathResolver('/index.md') + t.false(startsWith(result.file.ext, '.')) +}) diff --git a/test/utils/path-resolver.js b/test/utils/build-page/url-resolver.js similarity index 74% rename from test/utils/path-resolver.js rename to test/utils/build-page/url-resolver.js index bc9a1f34c6b38..9fb9bcf4dc268 100644 --- a/test/utils/path-resolver.js +++ b/test/utils/build-page/url-resolver.js @@ -1,58 +1,58 @@ import test from 'ava' -import pathResolver from '../../lib/utils/path-resolver' +import urlResolver from '../../../lib/utils/build-page/url-resolver' test('returns undefined when filename starts with _', t => { const parsedPath = { name: '_notapage' } const data = {} - const pagePath = pathResolver(data, parsedPath) + const pagePath = urlResolver(data, parsedPath) t.true(pagePath === undefined) }) test('uses the path from page data', t => { const parsedPath = { name: 'page', dirname: 'a-directory/' } const data = { path: '/page-at-root' } - const pagePath = pathResolver(data, parsedPath) + const pagePath = urlResolver(data, parsedPath) t.same(pagePath, '/page-at-root') }) test('removes index from path', t => { const parsedPath = { name: 'index', dirname: 'foo/bar' } const data = {} - const pagePath = pathResolver(data, parsedPath) + const pagePath = urlResolver(data, parsedPath) t.same(pagePath, '/foo/bar/') }) test('removes template from path', t => { const parsedPath = { name: 'template', dirname: 'foo/bar' } const data = {} - const pagePath = pathResolver(data, parsedPath) + const pagePath = urlResolver(data, parsedPath) t.same(pagePath, '/foo/bar/') }) test('template at root has root path', t => { const parsedPath = { name: 'template', dirname: '' } const data = {} - const pagePath = pathResolver(data, parsedPath) + const pagePath = urlResolver(data, parsedPath) t.same(pagePath, '/') }) test('index at root has root path', t => { const parsedPath = { name: 'index', dirname: '' } const data = {} - const pagePath = pathResolver(data, parsedPath) + const pagePath = urlResolver(data, parsedPath) t.same(pagePath, '/') }) test('if not an index create a path like /foo/bar/', t => { const parsedPath = { name: 'creating-static-websites', dirname: 'blog/2016' } const data = {} - const pagePath = pathResolver(data, parsedPath) + const pagePath = urlResolver(data, parsedPath) t.same(pagePath, '/blog/2016/creating-static-websites/') }) test('if not an index create a path like /foo/bar/', t => { const parsedPath = { name: 'about-us', dirname: '' } const data = {} - const pagePath = pathResolver(data, parsedPath) + const pagePath = urlResolver(data, parsedPath) t.same(pagePath, '/about-us/') }) From 3434e4913520652eaf79c72420194fd433053cfb Mon Sep 17 00:00:00 2001 From: Benjamin Stepp Date: Thu, 7 Apr 2016 21:35:16 -0500 Subject: [PATCH 3/3] Update ava to 0.14.0 --- package.json | 6 ++++-- test/utils/build-page/path-resolver.js | 16 ++++++++-------- test/utils/build-page/url-resolver.js | 14 +++++++------- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index f4a69717dcc5f..3e10bbea141a7 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,7 @@ "yaml-loader": "^0.1.0" }, "devDependencies": { - "ava": "^0.13.0", + "ava": "^0.14.0", "babel-cli": "^6.6.5", "babel-eslint": "^6.0.0", "eslint": "^2.4.0", @@ -114,7 +114,9 @@ "watch": "babel -w lib --out-dir dist/" }, "ava": { - "require": [ "babel-register" ], + "require": [ + "babel-register" + ], "babel": "inherit" } } diff --git a/test/utils/build-page/path-resolver.js b/test/utils/build-page/path-resolver.js index 28d0a4481428d..648aecb8633a4 100644 --- a/test/utils/build-page/path-resolver.js +++ b/test/utils/build-page/path-resolver.js @@ -6,14 +6,14 @@ import pathResolver from '../../../lib/utils/build-page/path-resolver' test('it returns an object', t => { const pagePath = '/' const result = pathResolver(pagePath) - t.ok(result) + t.truthy(result) t.is(typeof result, 'object') }) test('it has a require path', t => { const pagePath = '/a-blog-post/index.md' const result = pathResolver(pagePath) - t.ok(result.requirePath) + t.truthy(result.requirePath) t.is(typeof result.requirePath, 'string') }) @@ -24,15 +24,15 @@ test('it does not has a path if the name start with _', t => { }) test('it has a path if the name doesnt start with _', t => { - t.ok(pathResolver('/2015/back-to-the-future.md').path) - t.ok(pathResolver('/my-first-blog.md').path) - t.ok(pathResolver('/index.md').path) + t.truthy(pathResolver('/2015/back-to-the-future.md').path) + t.truthy(pathResolver('/my-first-blog.md').path) + t.truthy(pathResolver('/index.md').path) }) test('it has a templatePath if the name is _template', t => { const pagePath = '/_template.js' const result = pathResolver(pagePath) - t.ok(result.templatePath) + t.truthy(result.templatePath) t.is(typeof result.templatePath, 'string') }) @@ -46,8 +46,8 @@ test('the directory name has / slashes', t => { const pagePath = '/2016/testing-middleman-sites-with-capybara/index.md' const result = pathResolver(pagePath) - t.same(result.file.dirname, path.posix.normalize(result.file.dirname)) - t.same(result.file.dirname, '/2016/testing-middleman-sites-with-capybara') + t.is(result.file.dirname, path.posix.normalize(result.file.dirname)) + t.is(result.file.dirname, '/2016/testing-middleman-sites-with-capybara') }) test('the ext doesnt have a leading .', t => { diff --git a/test/utils/build-page/url-resolver.js b/test/utils/build-page/url-resolver.js index 9fb9bcf4dc268..358366a7ebec2 100644 --- a/test/utils/build-page/url-resolver.js +++ b/test/utils/build-page/url-resolver.js @@ -12,47 +12,47 @@ test('uses the path from page data', t => { const parsedPath = { name: 'page', dirname: 'a-directory/' } const data = { path: '/page-at-root' } const pagePath = urlResolver(data, parsedPath) - t.same(pagePath, '/page-at-root') + t.is(pagePath, '/page-at-root') }) test('removes index from path', t => { const parsedPath = { name: 'index', dirname: 'foo/bar' } const data = {} const pagePath = urlResolver(data, parsedPath) - t.same(pagePath, '/foo/bar/') + t.is(pagePath, '/foo/bar/') }) test('removes template from path', t => { const parsedPath = { name: 'template', dirname: 'foo/bar' } const data = {} const pagePath = urlResolver(data, parsedPath) - t.same(pagePath, '/foo/bar/') + t.is(pagePath, '/foo/bar/') }) test('template at root has root path', t => { const parsedPath = { name: 'template', dirname: '' } const data = {} const pagePath = urlResolver(data, parsedPath) - t.same(pagePath, '/') + t.is(pagePath, '/') }) test('index at root has root path', t => { const parsedPath = { name: 'index', dirname: '' } const data = {} const pagePath = urlResolver(data, parsedPath) - t.same(pagePath, '/') + t.is(pagePath, '/') }) test('if not an index create a path like /foo/bar/', t => { const parsedPath = { name: 'creating-static-websites', dirname: 'blog/2016' } const data = {} const pagePath = urlResolver(data, parsedPath) - t.same(pagePath, '/blog/2016/creating-static-websites/') + t.is(pagePath, '/blog/2016/creating-static-websites/') }) test('if not an index create a path like /foo/bar/', t => { const parsedPath = { name: 'about-us', dirname: '' } const data = {} const pagePath = urlResolver(data, parsedPath) - t.same(pagePath, '/about-us/') + t.is(pagePath, '/about-us/') })