From f80e0668db820fa6a77e48f765db82b346edf391 Mon Sep 17 00:00:00 2001
From: Christopher Garrett <opensource@pzuraq.com>
Date: Fri, 6 Apr 2018 17:33:41 -0700
Subject: [PATCH] [api] Remove helper items, rework navigation index

It turns out that helpers can actually be functions _or_ classes, which
was not something which the original data model took into account.
This PR removes the notion of a unique "helper" type from the data
model in favor of deferring to whatever construct is ultimately
resolved: Either a class or a function.

The navigation index code has also been simplified a bit - it was pretty
hacked together after EmberConf, so it needed some straightening out.
For now, the rule is that every resolvable file gets an entry, and the
the last segment of the file path becomes the entry's name. Helpers and
components are {{curlied}} and everything else is CapitalCased.

We are also no longer making assumptions about what a file is exporting.
This means that if  helper function or class is exported from resolved
directory, it will _not show up_ in the navigation at all. We can work
on solving this use case later on, but I think it will be sufficiently
rare that we can punt on it until later. `utils/` folders are
purposefully excluded from this and are placed instead in the module
structure, since that is the most common directory for helper functions.

We also need to consider how this structure may change with module
unification and nesting, but that's a problem for the future.
---
 addon/components/api/x-class/template.hbs     |   4 +-
 addon/components/api/x-component/template.hbs |   4 +-
 addon/components/api/x-module/template.hbs    |   2 -
 addon/models/module.js                        |   1 -
 lib/broccoli/docs-compiler.js                 | 118 ++++++++----------
 package.json                                  |   4 +-
 sandbox/app/helpers/esdoc-class-helper.js     |  18 +++
 sandbox/app/helpers/yuidoc-class-helper.js    |  23 ++++
 tests/acceptance/sandbox/api/helpers-test.js  |  83 ++++++++----
 tests/pages/api/class.js                      |  28 +++++
 yarn.lock                                     |   8 +-
 11 files changed, 188 insertions(+), 105 deletions(-)
 create mode 100644 sandbox/app/helpers/esdoc-class-helper.js
 create mode 100644 sandbox/app/helpers/yuidoc-class-helper.js
 create mode 100644 tests/pages/api/class.js

diff --git a/addon/components/api/x-class/template.hbs b/addon/components/api/x-class/template.hbs
index c226cfa6c..f2b5dc8b7 100644
--- a/addon/components/api/x-class/template.hbs
+++ b/addon/components/api/x-class/template.hbs
@@ -1,7 +1,7 @@
-<h1 class='docs-h1'>{{class.name}}</h1>
+<h1 class='docs-h1' data-test-class-name>{{class.name}}</h1>
 
 {{! wrapping in a div seems to work around https://github.com/ember-learn/ember-cli-addon-docs/issues/7 }}
-<div>{{{class.description}}}</div>
+<div data-test-class-description>{{{class.description}}}</div>
 
 {{#if hasContents}}
   <div class="flex flex-row-reverse">
diff --git a/addon/components/api/x-component/template.hbs b/addon/components/api/x-component/template.hbs
index 03d90a444..b9040361f 100644
--- a/addon/components/api/x-component/template.hbs
+++ b/addon/components/api/x-component/template.hbs
@@ -1,7 +1,7 @@
-<h1 class='docs-h1'>{{component.name}}</h1>
+<h1 class='docs-h1' data-test-component-name>{{component.name}}</h1>
 
 {{! wrapping in a div seems to work around https://github.com/ember-learn/ember-cli-addon-docs/issues/7 }}
-<div>{{{component.description}}}</div>
+<div data-test-component-name>{{{component.description}}}</div>
 
 {{#if hasContents}}
   <div class="flex flex-row-reverse">
diff --git a/addon/components/api/x-module/template.hbs b/addon/components/api/x-module/template.hbs
index 6f7a691a5..481b50d0c 100644
--- a/addon/components/api/x-module/template.hbs
+++ b/addon/components/api/x-module/template.hbs
@@ -4,7 +4,6 @@
       classes=module.classes
       components=module.components
       functions=module.functions
-      helpers=module.helpers
       variables=module.variables
     )
   }}
@@ -14,7 +13,6 @@
       classes=module.classes
       components=module.components
       functions=module.functions
-      helpers=module.helpers
       variables=module.variables
     )
   }}
diff --git a/addon/models/module.js b/addon/models/module.js
index c894240f9..5bdfba282 100644
--- a/addon/models/module.js
+++ b/addon/models/module.js
@@ -6,7 +6,6 @@ export default DS.Model.extend({
   file: attr(),
   variables: attr(),
   functions: attr(),
-  helpers: attr(),
 
   classes: hasMany('class', { async: false, }),
   components: hasMany('class', { async: false, })
diff --git a/lib/broccoli/docs-compiler.js b/lib/broccoli/docs-compiler.js
index a8d974490..118604307 100644
--- a/lib/broccoli/docs-compiler.js
+++ b/lib/broccoli/docs-compiler.js
@@ -57,7 +57,6 @@ function removeEmptyModules(modules) {
       module.classes.length === 0
       && module.components.length === 0
       && module.functions.length === 0
-      && module.helpers.length === 0
       && module.variables.length === 0
     );
   });
@@ -137,99 +136,82 @@ function hoistDefaults(modules) {
       delete modules[m];
     }
   }
+
+  return modules;
 }
 
-function generateResolvedTypedNavigationItems(collection, file, type) {
-  return collection.filter(c => c.file.includes(`${type}s/`)).map((c) => {
-    let segments = file.split('/');
+
+
+const RESOLVED_TYPES = [
+  'components',
+  'helpers',
+  'controllers',
+  'mixins',
+  'models',
+  'services'
+];
+
+function generateResolvedTypeNavigationItems(modules, type) {
+  let items = modules.map(m => {
+    let segments = m.file.split('/');
     let fileName = segments.pop();
 
-    if (fileName === type) {
+    if (type.match(fileName)) {
       fileName = segments.pop();
     }
 
     let name;
-    if (['component', 'helper'].includes(type)) {
+    if (['components', 'helpers'].includes(type)) {
       name = `{{${fileName}}}`;
     } else {
       name = _.upperFirst(_.camelCase(fileName));
     }
 
     return {
-      path: `${type}s/${fileName}`,
+      path: `${type}/${fileName}`,
       name
     };
   });
+
+  return _.sortBy(items, ['name']);
 }
 
-function generateModuleNavigationItems(module, collection) {
-  return collection.map((item) => {
-    return {
-      path: `root/${item.id || module.id}`,
-      name: item.name,
-      isDefault: item.exportType === 'default'
-    };
-  });
+function generateModuleNavigationItems(modules, type) {
+  let navItems = modules.reduce((navItems, m) => {
+    let items = m[type].map((item) => {
+      return {
+        path: `root/${item.id || module.id}`,
+        name: item.name,
+        isDefault: item.exportType === 'default'
+      };
+    });
+
+    if (items.length > 0) {
+        navItems[m.file] = _.sortBy(items, ['name']);
+    }
+
+    return navItems;
+  }, {});
+
+  return hoistDefaults(navItems);
 }
 
 function generateNavigationIndex(modules, klasses) {
-  let components = [];
-  let controllers = [];
-  let helpers = [];
-  let mixins = [];
-  let models = [];
-  let services = [];
-
-  let classes = {};
-  let functions = {};
-  let variables = {};
-
-  modules.forEach((m) => {
-    let file = m.file;
-
-    let componentItems = generateResolvedTypedNavigationItems(m.components, file, 'component');
-    let helperItems = generateResolvedTypedNavigationItems(m.helpers, file, 'helper');
-
-    let controllerItems = generateResolvedTypedNavigationItems(m.classes, file, 'controller');
-    let mixinItems = generateResolvedTypedNavigationItems(m.classes, file, 'mixin');
-    let modelItems = generateResolvedTypedNavigationItems(m.classes, file, 'model');
-    let serviceItems = generateResolvedTypedNavigationItems(m.classes, file, 'service');
-
-    let classItems = generateModuleNavigationItems(
-      m, m.classes.filter(c => !c.file.match(/controllers\/|mixins\/|models\/|services\//))
-    );
+  let navigationIndex = {};
 
-    let functionItems = generateModuleNavigationItems(m, m.functions);
-    let variableItems = generateModuleNavigationItems(m, m.variables);
+  for (let type of RESOLVED_TYPES) {
+    let resolvedModules = modules.filter(m => m.file.match(`${type}/`) && !m.file.match('utils/'));
 
-    components = components.concat(componentItems);
-    controllers = controllers.concat(controllerItems);
-    helpers = helpers.concat(helperItems);
-    mixins = mixins.concat(mixinItems);
-    models = models.concat(modelItems);
-    services = services.concat(serviceItems);
+    navigationIndex[type] = generateResolvedTypeNavigationItems(resolvedModules, type);
+  }
 
-    classes[file] = _.sortBy(classItems, ['name']);
-    functions[file] = _.sortBy(functionItems, ['name']);
-    variables[file] = _.sortBy(variableItems, ['name']);
-  });
+  let nonResolvedModules = modules.filter(m => {
+    return !m.file.match(new RegExp(`(${RESOLVED_TYPES.join('|')})/`)) || m.file.match('utils/');
+  })
 
-  hoistDefaults(classes);
-  hoistDefaults(functions);
-  hoistDefaults(variables);
-
-  let navigationIndex = {
-    components: _.sortBy(components, ['path']),
-    controllers: _.sortBy(controllers, ['name']),
-    helpers: _.sortBy(helpers, ['path']),
-    mixins: _.sortBy(mixins, ['name']),
-    models: _.sortBy(models, ['name']),
-    services: _.sortBy(services, ['name']),
-
-    classes,
-    functions,
-    variables
-  };
+  navigationIndex.classes = generateModuleNavigationItems(nonResolvedModules, 'classes');
+  navigationIndex.functions = generateModuleNavigationItems(nonResolvedModules, 'functions');
+  navigationIndex.variables = generateModuleNavigationItems(nonResolvedModules, 'variables');
 
   for (let type in navigationIndex) {
     let items = navigationIndex[type];
diff --git a/package.json b/package.json
index aa42a8d04..14e565bca 100644
--- a/package.json
+++ b/package.json
@@ -79,8 +79,8 @@
     "common-tags": "^1.7.2",
     "ember-classy-page-object": "^0.4.6",
     "ember-cli": "~3.0.0",
-    "ember-cli-addon-docs-esdoc": "^0.1.1",
-    "ember-cli-addon-docs-yuidoc": "^0.1.1",
+    "ember-cli-addon-docs-esdoc": "^0.2.0",
+    "ember-cli-addon-docs-yuidoc": "^0.2.0",
     "ember-cli-dependency-checker": "^2.1.0",
     "ember-cli-deploy": "^1.0.2",
     "ember-cli-deploy-build": "^1.1.1",
diff --git a/sandbox/app/helpers/esdoc-class-helper.js b/sandbox/app/helpers/esdoc-class-helper.js
new file mode 100644
index 000000000..f0c501ac9
--- /dev/null
+++ b/sandbox/app/helpers/esdoc-class-helper.js
@@ -0,0 +1,18 @@
+/** @documenter esdoc */
+
+import Helper from '@ember/component/helper';
+
+/**
+  A class based ESDoc helper
+ */
+export default class ESDocClassHelper extends Helper {
+  /**
+    returns the absolute value of a number
+
+    @param {number} [number] the passed number
+    @return {number}
+  */
+  compute([number]) {
+    return Math.abs(number);
+  }
+}
diff --git a/sandbox/app/helpers/yuidoc-class-helper.js b/sandbox/app/helpers/yuidoc-class-helper.js
new file mode 100644
index 000000000..7012801bd
--- /dev/null
+++ b/sandbox/app/helpers/yuidoc-class-helper.js
@@ -0,0 +1,23 @@
+/** @documenter yuidoc */
+
+import Helper from '@ember/component/helper';
+
+/**
+  A class based YUIDoc helper
+
+  @class YUIDocClassHelper
+ */
+const YUIDocClassHelper = Helper.extend({
+  /**
+    returns the absolute value of a number
+
+    @method compute
+    @param {number} [number] the passed number
+    @return {number}
+  */
+  compute([number]) {
+    return Math.abs(number);
+  }
+});
+
+export default YUIDocClassHelper;
diff --git a/tests/acceptance/sandbox/api/helpers-test.js b/tests/acceptance/sandbox/api/helpers-test.js
index 2af249ffc..6d0770986 100644
--- a/tests/acceptance/sandbox/api/helpers-test.js
+++ b/tests/acceptance/sandbox/api/helpers-test.js
@@ -3,43 +3,78 @@ import { setupApplicationTest } from 'ember-qunit';
 import { currentURL, visit } from '@ember/test-helpers';
 
 import modulePage from '../../../pages/api/module';
+import classPage from '../../../pages/api/class';
 
 module('Acceptance | API | helpers', function(hooks) {
   setupApplicationTest(hooks);
 
-  for (let documenter of ['esdoc', 'yuidoc']) {
-    let helperName = `${documenter}Helper`;
-    let kebabName = `${documenter}-helper`;
+  module('standard helpers', function() {
+    for (let documenter of ['esdoc', 'yuidoc']) {
+      let helperName = `${documenter}Helper`;
+      let kebabName = `${documenter}-helper`;
 
-    test('{{esdoc-helper}}', async function(assert) {
-      await visit('/sandbox');
-      await modulePage.navItems.findOne({ text: `{{${kebabName}}}` }).click();
+      test(`{{${kebabName}}}`, async function(assert) {
+        await visit('/sandbox');
+        await modulePage.navItems.findOne({ text: `{{${kebabName}}}` }).click();
 
-      assert.equal(currentURL(), `/sandbox/api/helpers/${kebabName}`, 'correct url');
+        assert.equal(currentURL(), `/sandbox/api/helpers/${kebabName}`, 'correct url');
 
-      let helpersSection = modulePage.sections.findOne({ header: 'Helpers' });
+        let functionsSection = modulePage.sections.findOne({ header: 'Functions' });
 
-      assert.ok(helpersSection.isPresent, 'Renders the helpers section');
+        assert.ok(functionsSection.isPresent, 'Renders the functions section');
 
-      let helperItem = helpersSection.items.findOne(i => i.header.includes(helperName));
+        let helperItem = functionsSection.items.findOne(i => i.header.includes(helperName));
 
-      assert.ok(helperItem.isPresent, 'Renders the helper item');
+        assert.ok(helperItem.isPresent, 'Renders the helper item');
 
-      assert.equal(
-        helperItem.header,
-        `${helperName}(number: number): number`,
-        'renders the type signature of the helper correctly'
-      );
+        assert.equal(
+          helperItem.header,
+          `${helperName}(number: number): number`,
+          'renders the type signature of the helper correctly'
+        );
 
-      assert.equal(
-        helperItem.importPath,
-        `import { ${helperName} } from 'ember-cli-addon-docsapp/helpers/${kebabName}';`,
-        'renders the import path correctly'
-      );
+        assert.equal(
+          helperItem.importPath,
+          `import { ${helperName} } from 'ember-cli-addon-docs/helpers/${kebabName}';`,
+          'renders the import path correctly'
+        );
 
-      assert.equal(helperItem.params.length, 1, 'renders the item parameter');
-    });
-  }
+        assert.equal(helperItem.params.length, 1, 'renders the item parameter');
+      });
+    }
+  });
+
+  module('class helpers', function() {
+    for (let documenter of ['ESDoc', 'YUIDoc']) {
+      let helperName = `${documenter}ClassHelper`;
+      let kebabName = `${documenter.toLowerCase()}-class-helper`;
+
+      test(`{{${kebabName}}}`, async function(assert) {
+        await visit('/sandbox');
+        await classPage.navItems.findOne({ text: `{{${kebabName}}}` }).click();
+
+        assert.equal(currentURL(), `/sandbox/api/helpers/${kebabName}`, 'correct url');
+
+        assert.equal(classPage.title, helperName, 'Renders the class title correctly');
+
+        let methodsSection = modulePage.sections.findOne({ header: 'Methods' });
+
+        assert.ok(methodsSection.isPresent, 'Renders the methods section');
+
+        let computeItem = methodsSection.items.findOne(i => i.header.includes('compute'));
+
+        assert.ok(computeItem.isPresent, 'Renders the helper item');
+
+        assert.equal(
+          computeItem.header,
+          'compute(number: number): number',
+          'renders the type signature of the helper correctly'
+        );
+
+        assert.equal(computeItem.params.length, 1, 'renders the item parameter');
+      });
+    }
+  });
 });
 
 
diff --git a/tests/pages/api/class.js b/tests/pages/api/class.js
new file mode 100644
index 000000000..2606d0a2a
--- /dev/null
+++ b/tests/pages/api/class.js
@@ -0,0 +1,28 @@
+import PageObject, { collection, text } from 'ember-classy-page-object';
+
+const ClassPage = PageObject.extend({
+  navItems: collection({ scope: '[data-test-id="nav-item"]' }),
+
+  title: text('[data-test-class-name]'),
+  description: text('[data-test-class-description]'),
+
+  sections: collection({
+    scope: '[data-test-api-section]',
+
+    header: text('[data-test-section-header]'),
+
+    items: collection({
+      scope: '[data-test-item]',
+
+      header: text('[data-test-item-header]'),
+      importPath: text('[data-test-import-path]'),
+      description: text('[data-test-item-description]'),
+
+      params: collection({
+        scope: '[data-test-item-params] [data-test-item-param]'
+      })
+    })
+  })
+});
+
+export default ClassPage.create();
diff --git a/yarn.lock b/yarn.lock
index cb231b564..243b8ade9 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -2934,9 +2934,9 @@ ember-classy-page-object@^0.4.6:
     ember-cli-babel "^6.6.0"
     ember-cli-page-object "1.13.0-alpha.1"
 
-ember-cli-addon-docs-esdoc@^0.1.1:
+ember-cli-addon-docs-esdoc@ember-learn/ember-cli-addon-docs-esdoc#5602769:
   version "0.1.1"
-  resolved "https://registry.yarnpkg.com/ember-cli-addon-docs-esdoc/-/ember-cli-addon-docs-esdoc-0.1.1.tgz#860ed79b2226dcc16fef2641350f9d5e3dcb6ae3"
+  resolved "https://codeload.github.com/ember-learn/ember-cli-addon-docs-esdoc/tar.gz/5602769c814aa364fe1b2a1de42f3042b2478c02"
   dependencies:
     broccoli-caching-writer "^3.0.3"
     ember-cli-babel "^6.6.0"
@@ -2949,9 +2949,9 @@ ember-cli-addon-docs-esdoc@^0.1.1:
     tmp "^0.0.33"
     walk-sync "^0.3.2"
 
-ember-cli-addon-docs-yuidoc@^0.1.1:
+ember-cli-addon-docs-yuidoc@ember-learn/ember-cli-addon-docs-yuidoc#cdf5dc5:
   version "0.1.1"
-  resolved "https://registry.yarnpkg.com/ember-cli-addon-docs-yuidoc/-/ember-cli-addon-docs-yuidoc-0.1.1.tgz#4c74a02839ca23c0a29d5677a5da5f7c310454e1"
+  resolved "https://codeload.github.com/ember-learn/ember-cli-addon-docs-yuidoc/tar.gz/cdf5dc5f64b8ac88d17bd136852acc1354a59a26"
   dependencies:
     broccoli-caching-writer "^3.0.3"
     ember-cli-babel "^6.6.0"