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

RFC: User Config file integration #53

Merged
merged 26 commits into from
Apr 30, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
76f9943
fix: adding config file read and integrate with context/webpack, addi…
hutchgrant Apr 23, 2019
5bfd5d7
fix: updating webpack.develop.js
hutchgrant Apr 23, 2019
25a921f
fix: remove dev output
hutchgrant Apr 23, 2019
88c95c1
docs: updating readme
hutchgrant Apr 23, 2019
654a80d
fix: removing additional output
hutchgrant Apr 23, 2019
14e1f25
fix: indentation 2 spaces
hutchgrant Apr 24, 2019
d349358
fix: .json to .js config files
hutchgrant Apr 26, 2019
590ced0
docs: chaning .json to .js readme
hutchgrant Apr 26, 2019
bb7e22d
Merge branch 'master' into rfc/user-config-issue-40
hutchgrant Apr 26, 2019
5d3f8c0
Merge branch 'master' into rfc/user-config-issue-40
hutchgrant Apr 28, 2019
b53fe73
test: moving greenwood config into mock-app
hutchgrant Apr 28, 2019
3053247
Merge branch 'rfc/user-config-issue-40' of github.com:ProjectEvergree…
hutchgrant Apr 28, 2019
3cd95cb
Merge branch 'master' of github.com:ProjectEvergreen/greenwood into r…
hutchgrant Apr 28, 2019
7528cea
fix: webpack config merge
hutchgrant Apr 28, 2019
cd810fe
Merge branch 'master' of github.com:ProjectEvergreen/greenwood into r…
hutchgrant Apr 28, 2019
12133d3
fix: config source paths in context
hutchgrant Apr 28, 2019
1891719
docs: root directory to workspace directory
hutchgrant Apr 28, 2019
10151c9
docs: add current working directory note
hutchgrant Apr 28, 2019
d7ead99
fix: adding config validation, changing source to workspace
hutchgrant Apr 28, 2019
42fbc3a
docs: updating readme for config workspace var
hutchgrant Apr 28, 2019
9fce093
test: update test for workspace instead of source
hutchgrant Apr 28, 2019
1a0c400
fix: adding additional check for if workspace exists
hutchgrant Apr 28, 2019
69d9ca4
docs: updating readme to note http:// default
hutchgrant Apr 28, 2019
7e195d8
fix: refactor config validation, some leftover merge cleanup
hutchgrant Apr 28, 2019
0a542ac
fix: tiny config refactor
hutchgrant Apr 28, 2019
ebb30ad
fix: check the devServer exists before checking keys
hutchgrant Apr 28, 2019
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
44 changes: 43 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ A modern and performant static site generator supporting Web Component based dev
## Getting Started
By default, Greenwood will generate a site for you in _public/_.
```shell
$ greenwood
$ greenwood build
```

Fun! But naturally you'll want to make your own pages. So create a folder called _src/pages/_ and create a page called _index.md_.
Expand Down Expand Up @@ -77,6 +77,14 @@ class index extends LitElement {
customElements.define('home-page', index);
```

### Development Mode

You can launch a development server with live-reload by running:

```bash
$ greenwood develop
```

## Advanced Markdown

You can also render custom html such as a custom style or even a component within your markdown page using `imports` in your front-matter variables at top, as well as utilizing the `render` code block e.g.
Expand All @@ -102,6 +110,40 @@ This is an example page built by Greenwood. Make your own in src/pages!
## API
Here are some of the features and capabiliites of Greenwood.

### Configure

Custom greenwood configurations can be added to a `greenwood.config.js` file in your root directory. For example, you may want to change the `src` folder to something else such as `www`.

```js
{
"source": "www"
}

```

#### PublicPath

If you're hosting at yourdomain.com/mysite/ as the root to your site, you can change the public path by adding it within a `greenwood.config.js`:

```js
{
"publicPath": "/mysite/",
}
```

#### Dev Server

You can adjust your dev server host and port, if you prefer to use something other than the default by adding it with a `greenwood.config.js`

```js
{
"devServer": {
"port": 1984,
"host": "localhost"
}
}
```

### Global CSS / Assets
> TODO
> https://github.com/ProjectEvergreen/greenwood/issues/7
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/config/webpack.config.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const mapUserWorkspaceDirectory = (userPath) => {
);
};

module.exports = (context) => {
module.exports = (context, config) => {
// dynamically map all the user's workspace directories for resolution by webpack
// this essentially helps us keep watch over changes from the user, and greenwood's build pipeline
const mappedUserDirectoriesForWebpack = getUserWorkspaceDirectories(context.userWorkspace).map(mapUserWorkspaceDirectory);
Expand All @@ -39,7 +39,7 @@ module.exports = (context) => {
output: {
path: context.publicDir,
filename: '[name].[hash].bundle.js',
publicPath: '/'
publicPath: config.publicPath
},

module: {
Expand Down
9 changes: 4 additions & 5 deletions packages/cli/config/webpack.config.develop.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ const generateCompilation = require('../lib/compile');
const webpackMerge = require('webpack-merge');
const commonConfig = require(path.join(__dirname, '..', './config/webpack.config.common.js'));

const host = 'localhost';
const port = 1981;
let isRebuilding = false;

const rebuild = async() => {
Expand All @@ -24,9 +22,10 @@ const rebuild = async() => {
}
};

module.exports = (context) => {
const configWithContext = commonConfig(context);
const publicPath = configWithContext.output.publicPath;
module.exports = ({ context, config }) => {
const configWithContext = commonConfig(context, config);
const { publicPath, devServer } = config;
const { host, port } = devServer;

return webpackMerge(configWithContext, {

Expand Down
6 changes: 3 additions & 3 deletions packages/cli/config/webpack.config.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ const path = require('path');
const webpackMerge = require('webpack-merge');
const commonConfig = require(path.join(__dirname, '..', './config/webpack.config.common.js'));

module.exports = (context) => {
const configWithContext = commonConfig(context);
module.exports = ({ context, config }) => {
const configWithContext = commonConfig(context, config);

return webpackMerge(configWithContext, {

Expand All @@ -18,7 +18,7 @@ module.exports = (context) => {
new HtmlWebpackPlugin({
filename: '404.html',
template: path.join(context.scratchDir, '404.html'),
publicPath: configWithContext.output.publicPath
publicPath: config.publicPath
})
]

Expand Down
17 changes: 9 additions & 8 deletions packages/cli/lib/compile.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,30 @@
require('colors');
const initConfig = require('./config');
const initContext = require('./init');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably not for this issue, but maybe we should create an issue to change init.js to context.js?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree with that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done
#57

const generateGraph = require('./graph');
const generateScaffolding = require('./scaffold');

// TODO would like to move graph and scaffold to the top more maybe?
module.exports = generateCompilation = () => {
return new Promise(async (resolve, reject) => {
try {

let compilation = {
graph: [],
context: {}
context: {},
config: {}
};

// read from defaults/config file
console.log('Reading project config');
compilation.config = await initConfig();

// determine whether to use default template or user detected workspace
console.log('Initializing project workspace contexts');
const context = await initContext(compilation);

compilation.context = context;
compilation.context = await initContext(compilation);

// generate a graph of all pages / components to build
console.log('Generating graph of workspace files...');
const graph = await generateGraph(compilation);

compilation.graph = graph;
compilation.graph = await generateGraph(compilation);

// generate scaffolding
console.log('Scaffolding out project files...');
Expand Down
33 changes: 33 additions & 0 deletions packages/cli/lib/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
const fs = require('fs');
const path = require('path');

let config = {
source: path.join(process.cwd(), 'src'),
devServer: {
port: 1984,
host: 'localhost'
},
publicPath: '/',
// TODO add global meta data see issue #5
// https://github.com/ProjectEvergreen/greenwood/issues/5
meta: {
title: '',
description: '',
author: '',
domain: ''
}
};

module.exports = readAndMergeConfig = async() => {
if (fs.existsSync(path.join(process.cwd(), 'greenwood.config.json'))) {
const userCfgFile = require(path.join(process.cwd(), 'greenwood.config.json'));

// prepend paths with current directory
if (userCfgFile.source) {
userCfgFile.source = path.join(process.cwd(), userCfgFile.source);
}

config = { ...config, ...userCfgFile };
}
return config;
};
30 changes: 15 additions & 15 deletions packages/cli/lib/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,26 @@ const fs = require('fs');
const path = require('path');
const greenwoodWorkspace = path.join(__dirname, '..');
const defaultTemplateDir = path.join(greenwoodWorkspace, 'templates/');
const defaultSrc = path.join(process.cwd(), 'src');

const userWorkspace = fs.existsSync(defaultSrc)
? defaultSrc
: defaultTemplateDir;

const pagesDir = fs.existsSync(path.join(userWorkspace, 'pages'))
? path.join(userWorkspace, 'pages/')
: defaultTemplateDir;

const templatesDir = fs.existsSync(path.join(userWorkspace, 'templates'))
? path.join(userWorkspace, 'templates/')
: defaultTemplateDir;

module.exports = initContexts = async() => {
module.exports = initContexts = async({ config }) => {

return new Promise((resolve, reject) => {

try {

const defaultSrc = config.source;

const userWorkspace = fs.existsSync(defaultSrc)
? defaultSrc
: defaultTemplateDir;

const pagesDir = fs.existsSync(path.join(userWorkspace, 'pages'))
? path.join(userWorkspace, 'pages/')
: defaultTemplateDir;

const templatesDir = fs.existsSync(path.join(userWorkspace, 'templates'))
? path.join(userWorkspace, 'templates/')
: defaultTemplateDir;

const context = {
userWorkspace,
pagesDir,
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/tasks/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ module.exports = runProductionBuild = async(compilation) => {
};

// eslint-disable-next-line no-unused-vars
const runWebpack = async ({ context }) => {
const webpackConfig = require(path.join(__dirname, '..', './config/webpack.config.prod.js'))(context);
const runWebpack = async (compilation) => {
const webpackConfig = require(path.join(__dirname, '..', './config/webpack.config.prod.js'))(compilation);

return new Promise(async (resolve, reject) => {

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/tasks/develop.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ const path = require('path');
const webpack = require('webpack');
const WebpackDevServer = require('webpack-dev-server');

module.exports = runDevServer = async ({ context }) => {
module.exports = runDevServer = async (compilation) => {
return new Promise(async (resolve, reject) => {

try {
const webpackConfig = require(path.join(__dirname, '..', './config/webpack.config.develop.js'))(context);
const webpackConfig = require(path.join(__dirname, '..', './config/webpack.config.develop.js'))(compilation);
const devServerConfig = webpackConfig.devServer;

let compiler = webpack(webpackConfig);
Expand Down
63 changes: 63 additions & 0 deletions test/cli.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,67 @@ describe('building greenwood with error handling for app and page templates', ()
await fs.remove(CONTEXT.scratchDir);
});

});

describe('building greenwood with user provided config file', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the intent is to also test with a nested directory output, lets add it to the describe description, otherwise I think we can drop those additional tests for nested pages since they are already covered above, and if our architecture is sound, anything that breaks nested directories here should in theory also break it up there, which is a better logical block to solve it in.

In essence, we can try and have tests that take advantage of other tests already testing for specific outputs, so tests can have minimal overlap while still breaking each other if something goes wrong when making a change.

Copy link
Member Author

@hutchgrant hutchgrant Apr 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They aren't covered under this scenario, thats why they are there. In theory they should break earlier in the tests, but not necessarily with all the tests.

Every single situation the public directory, js bundle, hello, nested, need to be tested.

When the source directory changes(which it does as per this test when a config file is copied), everything has to be retested or at least partially.

For example, had we tested the index route in the nested pages describe here(using user workspace mock-app), we would have caught the error for #54 which I just found today because one of the duplicate files which was in the correct path wasn't serialized.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the source directory changes(which it does as per this test when a config file is copied), everything has to be retested or at least partially.

Right good point.

And I think that's a good approach to take here, as for changing something like the userWorkspace, essentially we would want to run everything from the beginning.

One thing that isn't so clear here though, and maybe this could just be made more clear in a describe or it is the specific configuration used and making sure to assert on those specific values somehow?

I would expect to see specific tests to cover each config API being supported (userWorkspace, devServer), even if it means recreating a new config file or amending dynamically in some way to make it as explicit as possible, since we should make sure that if the user provides supported configuration items

  • sourceDirectory
  • devServer
  • etc

That we call those out specifically in our testing somehow, like though

  • return value of initConfig
  • output of file paths after running build
  • webpack config?

Although some cases may have some overlap, I think it's OK if it means that we can define and test the critical paths and outputs of a given feature / workflow / etc for the project if it ensures our tests can be as faithful to how a user would also setup / run our code. Providing a great and accurate development / testing environment we'll provide us much more grounded and confident in the usefulness and coverage of our tests.

A little duplication for the sake of clarity and setup is a positive tradeoff / investment in my book 👍

I always like to think to myself, the convenience in making build tools is to the user, not the maintainer.

before(async () => {
setup = new TestSetup();
CONTEXT = await setup.init();

// read user config file and copy it to app root
const userCfgFile = require(CONTEXT.userCfgPath);

await fs.copy(CONTEXT.userCfgPath, CONTEXT.userCfgRootPath);

// set new user source based on config file
CONTEXT.userSrc = path.join(__dirname, '..', userCfgFile.source);

// copy test app to configured source
await fs.copy(CONTEXT.testApp, CONTEXT.userSrc);
await setup.run(['./packages/cli/index.js', 'build']);

blogPageHtmlPath = path.join(CONTEXT.publicDir, 'blog', '20190326', 'index.html');
});

it('should output one JS bundle', async() => {
expect(await glob.promise(path.join(CONTEXT.publicDir, './**/index.*.bundle.js'))).to.have.lengthOf(1);
});

it('should contain a nested blog page directory', () => {
expect(fs.existsSync(path.join(CONTEXT.publicDir, 'blog', '20190326'))).to.be.true;
});

describe('nested generated blog page directory', () => {
const defaultHeading = 'Blog Page';
const defaultBody = 'This is the blog page built by Greenwood.';
let dom;

beforeEach(async() => {
dom = await JSDOM.fromFile(blogPageHtmlPath);
});

it('should contain a nested blog page with an index html file', () => {
expect(fs.existsSync(blogPageHtmlPath)).to.be.true;
});

it('should have the expected heading text within the blog page in the blog directory', async() => {
const heading = dom.window.document.querySelector('h3.wc-md-blog').textContent;

expect(heading).to.equal(defaultHeading);
});

it('should have the expected paragraph text within the blog page in the blog directory', async() => {
let paragraph = dom.window.document.querySelector('p.wc-md-blog').textContent;

expect(paragraph).to.equal(defaultBody);
});
});

after(async() => {
await fs.remove(CONTEXT.userSrc);
await fs.remove(CONTEXT.userCfgRootPath);
await fs.remove(CONTEXT.publicDir);
await fs.remove(CONTEXT.scratchDir);
});

});
9 changes: 9 additions & 0 deletions test/fixtures/greenwood.config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we set the indentation to 2 here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea I don't know why my editor defaults to 4, have to get around to changing that.

"source": "src3",
"publicPath": "/",
"devServer": {
"port": 1984,
"host": "localhost"
},
"meta": {}
}
6 changes: 4 additions & 2 deletions test/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ module.exports = class Setup {
init() {
return new Promise(async(resolve, reject) => {
try {
const ctx = await initContext();
const ctx = await initContext({ config: {} });
const context = {
...ctx,
userSrc: path.join(__dirname, '..', 'src'), // static src
userTemplates: path.join(__dirname, '..', 'src', 'templates'), // static src/templates for testing empty templates dir, redundant in #38
testApp: path.join(__dirname, 'fixtures', 'mock-app', 'src')
testApp: path.join(__dirname, 'fixtures', 'mock-app', 'src'),
userCfgPath: path.join(__dirname, 'fixtures', 'greenwood.config.json'),
userCfgRootPath: path.join(__dirname, '..', 'greenwood.config.json')
Copy link
Member

@thescientist13 thescientist13 Apr 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how come we are using this second config file? The names are really close and hard to distinguish. Maybe we just need more fixtures?

My comment here about creating "cases" of tests might be an appropriate justification.

Copy link
Member Author

@hutchgrant hutchgrant Apr 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

second config is just the hardcoded path to where the config will be copied. Hence: userCfgRootPath

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, so basically for mocking the user's project root, where their own greenwood.config.json would be located?

};

resolve(context);
Expand Down