Chore/issue 428 refactor smoke test and cases (#497)

* 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
ProjectEvergreen · Apr 3, 2021 · 4bc284f · 4bc284f
1 parent 16e19dd
commit 4bc284f
Show file tree

Hide file tree

Showing 71 changed files with 317 additions and 600 deletions.
diff --git a/greenwood.config.js b/greenwood.config.js
@@ -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');
@@ -26,7 +25,6 @@ module.exports = {
     { name: 'google-site-verification', content: '4rYd8k5aFD0jDnN0CCFgUXNe4eakLP4NnA18mNnK5P0' }
   ],
   plugins: [
-    // ...pluginImportCommonjs(),
     pluginGoogleAnalytics({
       analyticsId: 'UA-147204327-1'
     }),

diff --git a/nyc.config.js b/nyc.config.js
@@ -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'
   ],
 
@@ -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],

diff --git a/packages/cli/src/config/rollup.config.js b/packages/cli/src/config/rollup.config.js
@@ -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));
       }
@@ -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);
 
@@ -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);
 
@@ -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('../', './');
@@ -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({
@@ -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;
@@ -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
@@ -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, {
@@ -373,7 +357,6 @@ function greenwoodHtmlPlugin(compilation) {
               }
             });
 
-            // TODO this seems hacky; hardcoded dirs :D
             bundle.fileName = bundle.facadeModuleId.replace('.greenwood', 'public');
             bundle.code = newHtml;
           }
@@ -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, {
@@ -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) => {
@@ -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,

diff --git a/packages/cli/src/index.js b/packages/cli/src/index.js
@@ -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');
@@ -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('-------------------------------------------------------');
@@ -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')
@@ -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();
 }

diff --git a/packages/cli/src/lib/browser.js b/packages/cli/src/lib/browser.js
@@ -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 {

diff --git a/packages/cli/src/lib/router.js b/packages/cli/src/lib/router.js
@@ -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;
     }
   }
@@ -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) => {

diff --git a/packages/cli/src/lifecycles/compile.js b/packages/cli/src/lifecycles/compile.js
@@ -1,4 +1,3 @@
-// TODO require('colors');
 const initConfig = require('./config');
 const initContext = require('./context');
 const generateGraph = require('./graph');
@@ -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);
 

diff --git a/packages/cli/src/lifecycles/copy.js b/packages/cli/src/lifecycles/copy.js
@@ -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`);
 

diff --git a/packages/cli/src/lifecycles/serve.js b/packages/cli/src/lifecycles/serve.js
@@ -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);

diff --git a/packages/cli/src/plugins/resource/plugin-standard-font.js b/packages/cli/src/plugins/resource/plugin-standard-font.js
@@ -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) {

diff --git a/packages/cli/src/plugins/resource/plugin-standard-html.js b/packages/cli/src/plugins/resource/plugin-standard-html.js
@@ -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');
@@ -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>
@@ -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] === '/';
@@ -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);
@@ -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()

diff --git a/packages/cli/src/plugins/resource/plugin-standard-javascript.js b/packages/cli/src/plugins/resource/plugin-standard-javascript.js
@@ -28,8 +28,6 @@ class StandardJavaScriptResource extends ResourceInterface {
       }
     });
   }
-
-  // TODO pptional optimize w/ terser (not rollup)
 }
 
 module.exports = {

diff --git a/packages/cli/src/plugins/resource/plugin-standard-json.js b/packages/cli/src/plugins/resource/plugin-standard-json.js
@@ -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';
@@ -45,8 +43,6 @@ class StandardJsonResource extends ResourceInterface {
       }
     });
   }
-
-  // TODO include rollup json plugin support
 }
 
 module.exports = {
-Original file 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 @@