Skip to content

Commit

Permalink
Chore/issue 428 refactor smoke test and cases (#497)
Browse files Browse the repository at this point in the history
* refactor smoke test and re-enable

* cleanup dupe testing

* refactor beforeEach to before

* turn coverage checks back on

* delete invalid test cases

* xdescribe and xit

* fix smoke test tag matching and add smoke tests to ggraphql

* add more smoke testing

* add missing plugin-graphql ChildrenQuery spec

* cleanup TODOs for path references normalization

* remove TODOs that will be tracked via github

* clean up rollup todos

* restore local request blocking to puppeteer

* remove commented code from config

* final thresholds for now

* clean up missed TODOs and console logs
  • Loading branch information
thescientist13 committed Apr 3, 2021
1 parent 16e19dd commit 4bc284f
Show file tree
Hide file tree
Showing 71 changed files with 317 additions and 600 deletions.
2 changes: 0 additions & 2 deletions greenwood.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const path = require('path');
// const pluginImportCommonjs = require('./packages/plugin-import-commonjs/src/index');
const pluginGoogleAnalytics = require('./packages/plugin-google-analytics/src/index');
const pluginGraphQL = require('./packages/plugin-graphql/src/index');
const pluginImportCss = require('./packages/plugin-import-css/src/index');
Expand All @@ -26,7 +25,6 @@ module.exports = {
{ name: 'google-site-verification', content: '4rYd8k5aFD0jDnN0CCFgUXNe4eakLP4NnA18mNnK5P0' }
],
plugins: [
// ...pluginImportCommonjs(),
pluginGoogleAnalytics({
analyticsId: 'UA-147204327-1'
}),
Expand Down
12 changes: 6 additions & 6 deletions nyc.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ module.exports = {

include: [
'packages/cli/src/commands/*.js',
'packages/cli/src/data/*.js',
'packages/cli/src/lib/*.js',
'packages/cli/src/lifecycles/*.js',
'packages/cli/src/plugins/*.js',
'packages/plugin-*/src/*.js'
],

Expand All @@ -17,12 +17,12 @@ module.exports = {
'text-summary'
],

checkCoverage: false, // TODO renable
checkCoverage: true,

statements: 85,
branches: 75,
functions: 90,
lines: 85,
statements: 80,
branches: 65,
functions: 85,
lines: 80,

watermarks: {
statements: [75, 85],
Expand Down
29 changes: 4 additions & 25 deletions packages/cli/src/config/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ function greenwoodWorkspaceResolver (compilation) {
return {
name: 'greenwood-workspace-resolver',
resolveId(source) {
// TODO better way to handle relative paths? happens in generateBundle too
if ((source.indexOf('./') === 0 || source.indexOf('/') === 0) && path.extname(source) !== '.html' && fs.existsSync(path.join(userWorkspace, source))) {
return source.replace(source, path.join(userWorkspace, source));
}
Expand Down Expand Up @@ -126,7 +125,6 @@ function greenwoodHtmlPlugin(compilation) {
: null;
}))).filter(resource => resource);

// TODO should reduce here instead
if (resourceHandler.length) {
const response = await resourceHandler[0].serve(id);

Expand All @@ -153,8 +151,7 @@ function greenwoodHtmlPlugin(compilation) {
});
const headScripts = root.querySelectorAll('script');
const headLinks = root.querySelectorAll('link');

// TODO handle deeper paths. e.g. ../../../

headScripts.forEach((scriptTag) => {
const parsedAttributes = parseTagForAttributes(scriptTag);

Expand All @@ -165,9 +162,6 @@ function greenwoodHtmlPlugin(compilation) {
} else {
const { src } = parsedAttributes;

// TODO avoid using href and set it to the value of rollup fileName instead
// since user paths can still be the same file,
// e.g. ../theme.css and ./theme.css are still the same file
mappedScripts.set(src, true);

const srcPath = src.replace('../', './');
Expand Down Expand Up @@ -196,12 +190,7 @@ function greenwoodHtmlPlugin(compilation) {
${scriptTag.rawText}
`.trim();

// have to write a file for rollup?
fs.writeFileSync(path.join(scratchDir, filename), source);

// TODO avoid using href and set it to the value of rollup fileName instead
// since user paths can still be the same file,
// e.g. ../theme.css and ./theme.css are still the same file
mappedScripts.set(id, true);

this.emitFile({
Expand All @@ -225,7 +214,6 @@ function greenwoodHtmlPlugin(compilation) {
href = href.slice(1);
}

// TODO handle auto expanding deeper paths
const basePath = href.indexOf(tokenNodeModules) >= 0
? projectDirectory
: userWorkspace;
Expand All @@ -244,9 +232,6 @@ function greenwoodHtmlPlugin(compilation) {
});
}

// TODO avoid using href and set it to the value of rollup fileName instead
// since user paths can still be the same file,
// e.g. ../theme.css and ./theme.css are still the same file
mappedStyles[parsedAttributes.href] = {
type: 'asset',
fileName: fileName.indexOf(tokenNodeModules) >= 0
Expand Down Expand Up @@ -298,7 +283,6 @@ function greenwoodHtmlPlugin(compilation) {
try {
const bundle = bundles[bundleId];

// TODO handle (!) Generated empty chunks .greenwood/about, .greenwood/index
if (bundle.isEntry && path.extname(bundle.facadeModuleId) === '.html') {
const html = fs.readFileSync(bundle.facadeModuleId, 'utf-8');
const root = htmlparser.parse(html, {
Expand Down Expand Up @@ -373,7 +357,6 @@ function greenwoodHtmlPlugin(compilation) {
}
});

// TODO this seems hacky; hardcoded dirs :D
bundle.fileName = bundle.facadeModuleId.replace('.greenwood', 'public');
bundle.code = newHtml;
}
Expand All @@ -390,7 +373,6 @@ function greenwoodHtmlPlugin(compilation) {
const bundle = bundles[bundleId];

if (bundle.isEntry && path.extname(bundle.facadeModuleId) === '.html') {
// TODO this seems hacky; hardcoded dirs :D
const htmlPath = bundle.facadeModuleId.replace('.greenwood', 'public');
let html = fs.readFileSync(htmlPath, 'utf-8');
const root = htmlparser.parse(html, {
Expand Down Expand Up @@ -478,17 +460,15 @@ function greenwoodHtmlPlugin(compilation) {
module.exports = getRollupConfig = async (compilation) => {
const { scratchDir, outputDir } = compilation.context;
const defaultRollupPlugins = [
// TODO replace should come in via plugin-node-modules
replace({ // https://github.com/rollup/rollup/issues/487#issuecomment-177596512
'process.env.NODE_ENV': JSON.stringify('production')
}),
nodeResolve(), // TODO move to plugin-node-modules
nodeResolve(),
greenwoodWorkspaceResolver(compilation),
greenwoodHtmlPlugin(compilation),
multiInput(),
json() // TODO make it part plugin-standard-json
json()
];
// TODO greenwood standard plugins, then "Greenwood" plugins, then user plugins
const customRollupPlugins = compilation.config.plugins.filter((plugin) => {
return plugin.type === 'rollup';
}).map((plugin) => {
Expand All @@ -497,12 +477,11 @@ module.exports = getRollupConfig = async (compilation) => {

if (compilation.config.optimization !== 'none') {
defaultRollupPlugins.push(
terser() // TODO extract to plugin-standard-javascript
terser()
);
}

return [{
// TODO Avoid .greenwood/ directory, do everything in public/?
input: `${scratchDir}**/*.html`,
output: {
dir: outputDir,
Expand Down
8 changes: 0 additions & 8 deletions packages/cli/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
// https://github.com/ProjectEvergreen/greenwood/issues/141
process.setMaxListeners(0);

// TODO require('colors');

const program = require('commander');
const runProductionBuild = require('./commands/build');
const runDevServer = require('./commands/develop');
Expand All @@ -17,10 +15,6 @@ const greenwoodPackageJson = require('../package.json');
let cmdOption = {};
let command = '';

// TODO
// console.log(`${chalk.rgb(175, 207, 71)('-------------------------------------------------------')}`);
// console.log(`${chalk.rgb(175, 207, 71)('Welcome to Greenwood ♻️')}`);
// console.log(`${chalk.rgb(175, 207, 71)('-------------------------------------------------------')}`);
console.info('-------------------------------------------------------');
console.info('Welcome to Greenwood ♻️');
console.info('-------------------------------------------------------');
Expand All @@ -29,7 +23,6 @@ program
.version(greenwoodPackageJson.version)
.arguments('<script-mode>')
.usage('<script-mode> [options]');
// TODO .usage(`${chalk.green('<script-mode>')} [options]`);

program
.command('build')
Expand Down Expand Up @@ -63,7 +56,6 @@ program

program.parse(process.argv);

// TODO pick build by default? Thinking of npx usage...
if (program.parse.length === 0) {
program.help();
}
Expand Down
28 changes: 12 additions & 16 deletions packages/cli/src/lib/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,19 @@ class BrowserRunner {

await page.setRequestInterception(true);

// only allow puppeteer to load necessary scripts needed for pre-rendering of the site itself
// only allow puppeteer to load necessary (local) scripts needed for pre-rendering of the site itself
page.on('request', interceptedRequest => {
// const interceptedRequestUrl = interceptedRequest.url();

// console.debug('request', interceptedRequestUrl);
// TODO handle serialize only requests
interceptedRequest.continue();
// if (
// interceptedRequestUrl.indexOf('bundle.js') >= 0 || // webpack bundles, webcomponents-bundle.js
// interceptedRequestUrl === requestUrl || // pages / routes
// interceptedRequestUrl.indexOf('localhost:4000') >= 0 // Apollo GraphQL server
// ) {
// interceptedRequest.continue();
// } else {
// console.debug('aborting request', interceptedRequestUrl)
// interceptedRequest.abort();
// }
const interceptedRequestUrl = interceptedRequest.url();

if (
interceptedRequestUrl.indexOf('http://127.0.0.1') >= 0 ||
interceptedRequestUrl.indexOf('localhost') >= 0
) {
interceptedRequest.continue();
} else {
// console.warn('aborting request', interceptedRequestUrl);
interceptedRequest.abort();
}
});

try {
Expand Down
7 changes: 1 addition & 6 deletions packages/cli/src/lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,16 @@ document.addEventListener('click', function(e) {
e.preventDefault();

if (e.path[0].href) {
console.debug('linked clicked was...', e.path[0].href);
const target = e.path[0];
const route = target.href.replace(window.location.origin, '');
const routerOutlet = Array.from(document.getElementsByTagName('greenwood-route')).filter(outlet => {
return outlet.getAttribute('data-route') === route;
})[0];

console.debug('routerOutlet', routerOutlet);

if (routerOutlet.getAttribute('data-template') === window.__greenwood.currentTemplate) {
window.__greenwood.currentTemplate = routerOutlet.getAttribute('data-template');
routerOutlet.loadRoute();
} else {
console.debug('new template detected, should do a hard reload');
window.location.href = target;
}
}
Expand All @@ -26,8 +22,7 @@ class RouteComponent extends HTMLElement {
loadRoute() {
const route = this.getAttribute('data-route');
const key = this.getAttribute('data-key');
console.debug('load route ->', route);
console.debug('with bundle ->', key);

fetch(key)
.then(res => res.text())
.then((response) => {
Expand Down
2 changes: 0 additions & 2 deletions packages/cli/src/lifecycles/compile.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// TODO require('colors');
const initConfig = require('./config');
const initContext = require('./context');
const generateGraph = require('./graph');
Expand All @@ -21,7 +20,6 @@ module.exports = generateCompilation = () => {
compilation.context = await initContext(compilation);

// generate a graph of all pages / components to build
// TODO make this async somehow / run in parallel?
console.info('Generating graph of workspace files...');
compilation = await generateGraph(compilation);

Expand Down
1 change: 0 additions & 1 deletion packages/cli/src/lifecycles/copy.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ module.exports = copyAssets = (compilation) => {
}
}

// TODO should be done via rollup detection, so Greenwood will only copy files used when actually imported by the user
console.info('copying graph.json...');
await copyFile(`${context.scratchDir}graph.json`, `${context.outputDir}/graph.json`);

Expand Down
1 change: 0 additions & 1 deletion packages/cli/src/lifecycles/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ function getProdServer(compilation) {
ctx.body = contents;
}

// TODO break up into distinct font / icons / svg handlers, decouple from to assets/
if (url.indexOf('assets/')) {
const assetPath = path.join(outputDir, url);
const ext = path.extname(assetPath);
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/plugins/resource/plugin-standard-font.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const { ResourceInterface } = require('../../lib/resource-interface');
class StandardFontResource extends ResourceInterface {
constructor(compilation, options) {
super(compilation, options);
this.extensions = ['.woff2', '.woff', '.ttf']; // TODO support more types?
this.extensions = ['.woff2', '.woff', '.ttf'];
}

async serve(url) {
Expand Down
6 changes: 0 additions & 6 deletions packages/cli/src/plugins/resource/plugin-standard-html.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const remarkRehype = require('remark-rehype');
const { ResourceInterface } = require('../../lib/resource-interface');
const unified = require('unified');

// TODO better error handling / messaging for users if things are not where they are expected to be
// general refactoring
const getPageTemplate = (barePath, workspace, template) => {
const templatesDir = path.join(workspace, 'templates');
Expand Down Expand Up @@ -118,8 +117,6 @@ const getAppTemplate = (contents, userWorkspace) => {

const getUserScripts = (contents) => {
if (process.env.__GWD_COMMAND__ === 'build') { // eslint-disable-line no-underscore-dangle
// TODO setup and teardown should be done together
// console.debug('running in build mode, polyfill WebComponents for puppeteer');
contents = contents.replace('<head>', `
<head>
<script src="/node_modules/@webcomponents/webcomponentsjs/webcomponents-bundle.js"></script>
Expand All @@ -133,7 +130,6 @@ const getMetaContent = (url, config, contents) => {
const metaContent = config.meta.map(item => {
let metaHtml = '';

// TODO better way to implememnt this? should we implement this?
for (const [key, value] of Object.entries(item)) {
const isOgUrl = item.property === 'og:url' && key === 'content';
const hasTrailingSlash = isOgUrl && value[value.length - 1] === '/';
Expand All @@ -151,7 +147,6 @@ const getMetaContent = (url, config, contents) => {
: `<meta${metaHtml}/>`;
}).join('\n');

// TODO make smarter so that if it already exists, then leave it alone
contents = contents.replace(/<title>(.*)<\/title>/, '');
contents = contents.replace('<head>', `<head><title>${title}</title>`);
contents = contents.replace('<meta-outlet></meta-outlet>', metaContent);
Expand Down Expand Up @@ -219,7 +214,6 @@ class StandardHtmlResource extends ResourceInterface {
}
});

// TODO extract front matter contents from remark-frontmatter instead of frontmatter lib
const settings = config.markdown.settings || {};
const fm = frontmatter(markdownContents);
processedMarkdown = await unified()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ class StandardJavaScriptResource extends ResourceInterface {
}
});
}

// TODO pptional optimize w/ terser (not rollup)
}

module.exports = {
Expand Down
4 changes: 0 additions & 4 deletions packages/cli/src/plugins/resource/plugin-standard-json.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@ class StandardJsonResource extends ResourceInterface {
let contentType = '';
const { context } = this.compilation;

// TODO should be its own plugin / package, as part of data
if (url.indexOf('graph.json') >= 0) {
const json = await fs.promises.readFile(path.join(context.scratchDir, 'graph.json'), 'utf-8');

contentType = 'application/json';
body = JSON.parse(json);
} else {
// TODO should be its own plugin / package
const json = await fs.promises.readFile(url, 'utf-8');

contentType = 'text/javascript';
Expand All @@ -45,8 +43,6 @@ class StandardJsonResource extends ResourceInterface {
}
});
}

// TODO include rollup json plugin support
}

module.exports = {
Expand Down
Loading

0 comments on commit 4bc284f

Please sign in to comment.