From e9f4f1b416c04e2622e8d09e665ed03b7b16609e Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sat, 8 Sep 2018 21:57:46 +0100 Subject: [PATCH] build(docs-infra): process and render ngmodule exports (#25734) All directives and pipes must now be tagged with one ore more public NgModule, from which they are exported. If an item is exported transitively via a re-exported internal NgModule then it may be that the item appears to be exported from more than one public NgModule. For example, there are shared directives that are exported in this way from `FormsModule` and `ReactiveFormsModule`. The doc-gen will error and fail if a directive or pipe is not tagged correctly. NgModule pages now list all the directives and pipes that are exported from it. Directive and Pipe pages now list any NgModule from which they are exported. Packages also now list any NgModules that are contained - previously they were missed. PR Close #25734 --- aio/src/styles/2-modules/_api-pages.scss | 7 +- .../processors/processNgModuleDocs.js | 43 +++++++- .../processors/processNgModuleDocs.spec.js | 101 +++++++++++++++++- .../processors/processPackages.js | 1 + .../angular-api-package/tag-defs/ngModule.js | 6 +- .../templates/api/directive.template.html | 3 +- .../templates/api/includes/ngmodule.html | 11 ++ .../templates/api/includes/pipe-overview.html | 2 + .../templates/api/ngmodule.template.html | 32 +++++- .../templates/api/package.template.html | 1 + 10 files changed, 196 insertions(+), 11 deletions(-) create mode 100644 aio/tools/transforms/templates/api/includes/ngmodule.html diff --git a/aio/src/styles/2-modules/_api-pages.scss b/aio/src/styles/2-modules/_api-pages.scss index 55e273408f..8443bf5bca 100644 --- a/aio/src/styles/2-modules/_api-pages.scss +++ b/aio/src/styles/2-modules/_api-pages.scss @@ -102,10 +102,15 @@ clear: left; } } -} + } .from-constructor, .read-only-property { font-style: italic; color: $blue; } + + .ngmodule-list { + list-style: none; + padding: 0; + } } diff --git a/aio/tools/transforms/angular-api-package/processors/processNgModuleDocs.js b/aio/tools/transforms/angular-api-package/processors/processNgModuleDocs.js index 936bb9b5be..bc0c98f3d7 100644 --- a/aio/tools/transforms/angular-api-package/processors/processNgModuleDocs.js +++ b/aio/tools/transforms/angular-api-package/processors/processNgModuleDocs.js @@ -1,7 +1,7 @@ -module.exports = function processNgModuleDocs() { +module.exports = function processNgModuleDocs(getDocFromAlias, createDocMessage, log) { return { - $runAfter: ['extractDecoratedClassesProcessor'], - $runBefore: ['docs-processed'], + $runAfter: ['extractDecoratedClassesProcessor', 'computeIdsProcessor'], + $runBefore: ['createSitemap'], $process(docs) { docs.forEach(doc => { if (doc.docType === 'ngmodule') { @@ -13,6 +13,43 @@ module.exports = function processNgModuleDocs() { }); } }); + + // Match all the directives/pipes to their module + const errors = []; + docs.forEach(doc => { + if (['directive', 'pipe'].indexOf(doc.docType) !== -1) { + if (!doc.ngModules || doc.ngModules.length === 0) { + errors.push(createDocMessage(`"${doc.id}" has no @ngModule tag. Docs of type "${doc.docType}" must have this tag.`, doc)); + return; + } + + doc.ngModules.forEach((ngModule, index) => { + + const ngModuleDocs = getDocFromAlias(ngModule, doc); + + if (ngModuleDocs.length === 0) { + errors.push(createDocMessage(`"@ngModule ${ngModule}" does not match a public NgModule`, doc)); + return; + } + + if (ngModuleDocs.length > 1) { + errors.push(createDocMessage(`"@ngModule ${ngModule}" is ambiguous. Matches: ${ngModuleDocs.map(d => d.id).join(', ')}`, doc)); + return; + } + + const ngModuleDoc = ngModuleDocs[0]; + const container = ngModuleDoc[doc.docType + 's'] = ngModuleDoc[doc.docType + 's'] || []; + container.push(doc); + + doc.ngModules[index] = ngModuleDoc; + }); + } + }); + + if (errors.length) { + errors.forEach(error => log.error(error)); + throw new Error('Failed to process NgModule relationships.'); + } } }; }; diff --git a/aio/tools/transforms/angular-api-package/processors/processNgModuleDocs.spec.js b/aio/tools/transforms/angular-api-package/processors/processNgModuleDocs.spec.js index 49d1d20468..24f72c91cc 100644 --- a/aio/tools/transforms/angular-api-package/processors/processNgModuleDocs.spec.js +++ b/aio/tools/transforms/angular-api-package/processors/processNgModuleDocs.spec.js @@ -3,9 +3,10 @@ const Dgeni = require('dgeni'); describe('processNgModuleDocs processor', () => { let processor; + let injector; beforeEach(() => { const dgeni = new Dgeni([testPackage('angular-api-package')]); - const injector = dgeni.configureInjector(); + injector = dgeni.configureInjector(); processor = injector.get('processNgModuleDocs'); }); @@ -14,11 +15,105 @@ describe('processNgModuleDocs processor', () => { }); it('should run before the correct processor', () => { - expect(processor.$runBefore).toEqual(['docs-processed']); + expect(processor.$runBefore).toEqual(['createSitemap']); }); it('should run after the correct processor', () => { - expect(processor.$runAfter).toEqual(['extractDecoratedClassesProcessor']); + expect(processor.$runAfter).toEqual(['extractDecoratedClassesProcessor', 'computeIdsProcessor']); }); + it('should non-arrayNgModule options to arrays', () => { + const docs = [{ + docType: 'ngmodule', + ngmoduleOptions: { + a: ['AAA'], + b: 'BBB', + c: 42 + } + }]; + processor.$process(docs); + expect(docs[0].ngmoduleOptions.a).toEqual(['AAA']); + expect(docs[0].ngmoduleOptions.b).toEqual(['BBB']); + expect(docs[0].ngmoduleOptions.c).toEqual([42]); + }); + + it('should link directive/pipe docs with their NgModule docs', () => { + const aliasMap = injector.get('aliasMap'); + const ngModule1 = { docType: 'ngmodule', id: 'NgModule1', aliases: ['NgModule1'], ngmoduleOptions: {}}; + const ngModule2 = { docType: 'ngmodule', id: 'NgModule2', aliases: ['NgModule2'], ngmoduleOptions: {}}; + const directive1 = { docType: 'directive', id: 'Directive1', ngModules: ['NgModule1']}; + const directive2 = { docType: 'directive', id: 'Directive2', ngModules: ['NgModule2']}; + const directive3 = { docType: 'directive', id: 'Directive3', ngModules: ['NgModule1', 'NgModule2']}; + const pipe1 = { docType: 'pipe', id: 'Pipe1', ngModules: ['NgModule1']}; + const pipe2 = { docType: 'pipe', id: 'Pipe2', ngModules: ['NgModule2']}; + const pipe3 = { docType: 'pipe', id: 'Pipe3', ngModules: ['NgModule1', 'NgModule2']}; + + aliasMap.addDoc(ngModule1); + aliasMap.addDoc(ngModule2); + processor.$process([ngModule1, ngModule2, directive1, directive2, directive3, pipe1, pipe2, pipe3]); + + expect(ngModule1.directives).toEqual([directive1, directive3]); + expect(ngModule1.pipes).toEqual([pipe1, pipe3]); + expect(ngModule2.directives).toEqual([directive2, directive3]); + expect(ngModule2.pipes).toEqual([pipe2, pipe3]); + + expect(directive1.ngModules).toEqual([ngModule1]); + expect(directive2.ngModules).toEqual([ngModule2]); + expect(directive3.ngModules).toEqual([ngModule1, ngModule2]); + + expect(pipe1.ngModules).toEqual([ngModule1]); + expect(pipe2.ngModules).toEqual([ngModule2]); + expect(pipe3.ngModules).toEqual([ngModule1, ngModule2]); + }); + + it('should error if a pipe/directive does not have a `@ngModule` tag', () => { + const log = injector.get('log'); + expect(() => { + processor.$process([{ docType: 'directive', id: 'Directive1' }]); + }).toThrowError('Failed to process NgModule relationships.'); + expect(log.error).toHaveBeenCalledWith( + '"Directive1" has no @ngModule tag. Docs of type "directive" must have this tag. - doc "Directive1" (directive) '); + + expect(() => { + processor.$process([{ docType: 'pipe', id: 'Pipe1' }]); + }).toThrowError('Failed to process NgModule relationships.'); + expect(log.error).toHaveBeenCalledWith( + '"Pipe1" has no @ngModule tag. Docs of type "pipe" must have this tag. - doc "Pipe1" (pipe) '); + }); + + it('should error if a pipe/directive has an @ngModule tag that does not match an NgModule doc', () => { + const log = injector.get('log'); + expect(() => { + processor.$process([{ docType: 'directive', id: 'Directive1', ngModules: ['MissingNgModule'] }]); + }).toThrowError('Failed to process NgModule relationships.'); + expect(log.error).toHaveBeenCalledWith( + '"@ngModule MissingNgModule" does not match a public NgModule - doc "Directive1" (directive) '); + + expect(() => { + processor.$process([{ docType: 'pipe', id: 'Pipe1', ngModules: ['MissingNgModule'] }]); + }).toThrowError('Failed to process NgModule relationships.'); + expect(log.error).toHaveBeenCalledWith( + '"@ngModule MissingNgModule" does not match a public NgModule - doc "Pipe1" (pipe) '); + }); + + it('should error if a pipe/directive has an @ngModule tag that matches more than one NgModule doc', () => { + const aliasMap = injector.get('aliasMap'); + const log = injector.get('log'); + const ngModule1 = { docType: 'ngmodule', id: 'NgModule1', aliases: ['NgModuleAlias'], ngmoduleOptions: {}}; + const ngModule2 = { docType: 'ngmodule', id: 'NgModule2', aliases: ['NgModuleAlias'], ngmoduleOptions: {}}; + aliasMap.addDoc(ngModule1); + aliasMap.addDoc(ngModule2); + + expect(() => { + processor.$process([{ docType: 'directive', id: 'Directive1', ngModules: ['NgModuleAlias'] }]); + }).toThrowError('Failed to process NgModule relationships.'); + expect(log.error).toHaveBeenCalledWith( + '"@ngModule NgModuleAlias" is ambiguous. Matches: NgModule1, NgModule2 - doc "Directive1" (directive) '); + + expect(() => { + processor.$process([{ docType: 'pipe', id: 'Pipe1', ngModules: ['NgModuleAlias'] }]); + }).toThrowError('Failed to process NgModule relationships.'); + expect(log.error).toHaveBeenCalledWith( + '"@ngModule NgModuleAlias" is ambiguous. Matches: NgModule1, NgModule2 - doc "Pipe1" (pipe) '); + }); }); diff --git a/aio/tools/transforms/angular-api-package/processors/processPackages.js b/aio/tools/transforms/angular-api-package/processors/processPackages.js index 3d9fc3f202..e4124155f7 100644 --- a/aio/tools/transforms/angular-api-package/processors/processPackages.js +++ b/aio/tools/transforms/angular-api-package/processors/processPackages.js @@ -26,6 +26,7 @@ module.exports = function processPackages() { // Partition the exports into groups by type if (doc.exports) { + doc.ngmodules = doc.exports.filter(doc => doc.docType === 'ngmodule'); doc.classes = doc.exports.filter(doc => doc.docType === 'class'); doc.decorators = doc.exports.filter(doc => doc.docType === 'decorator'); doc.functions = doc.exports.filter(doc => doc.docType === 'function'); diff --git a/aio/tools/transforms/angular-api-package/tag-defs/ngModule.js b/aio/tools/transforms/angular-api-package/tag-defs/ngModule.js index a80c7df32a..a1e9dd36f1 100644 --- a/aio/tools/transforms/angular-api-package/tag-defs/ngModule.js +++ b/aio/tools/transforms/angular-api-package/tag-defs/ngModule.js @@ -1,3 +1,7 @@ module.exports = function() { - return {name: 'ngModule'}; + return { + name: 'ngModule', + docProperty: 'ngModules', + multi: true, + }; }; diff --git a/aio/tools/transforms/templates/api/directive.template.html b/aio/tools/transforms/templates/api/directive.template.html index 79c9fc8752..daeae4e526 100644 --- a/aio/tools/transforms/templates/api/directive.template.html +++ b/aio/tools/transforms/templates/api/directive.template.html @@ -4,7 +4,8 @@ {% block overview %}{% include "includes/directive-overview.html" %}{% endblock %} {% block additional -%} - {% include "includes/selectors.html" %} +{% include "includes/ngmodule.html" %} +{% include "includes/selectors.html" %} {$ directiveHelper.renderBindings(doc.inputs, 'inputs', 'input', 'Inputs') $} {$ directiveHelper.renderBindings(doc.outputs, 'outputs', 'output', 'Outputs') $} {% include "includes/export-as.html" %} diff --git a/aio/tools/transforms/templates/api/includes/ngmodule.html b/aio/tools/transforms/templates/api/includes/ngmodule.html new file mode 100644 index 0000000000..a3421b9bed --- /dev/null +++ b/aio/tools/transforms/templates/api/includes/ngmodule.html @@ -0,0 +1,11 @@ +

NgModules

+ + diff --git a/aio/tools/transforms/templates/api/includes/pipe-overview.html b/aio/tools/transforms/templates/api/includes/pipe-overview.html index ec822379dd..03b00ea29c 100644 --- a/aio/tools/transforms/templates/api/includes/pipe-overview.html +++ b/aio/tools/transforms/templates/api/includes/pipe-overview.html @@ -10,6 +10,8 @@ {%- if param.isOptional or param.defaultValue !== undefined %} ]{% endif %} {%- endfor %} }} + {% include "includes/ngmodule.html" %} + {% if doc.valueParam.type %}

Input value

{$ params.renderParameters([doc.valueParam], 'pipe-parameters', 'pipe-parameter', true) $} diff --git a/aio/tools/transforms/templates/api/ngmodule.template.html b/aio/tools/transforms/templates/api/ngmodule.template.html index b20f91d3b0..32e1d74d12 100644 --- a/aio/tools/transforms/templates/api/ngmodule.template.html +++ b/aio/tools/transforms/templates/api/ngmodule.template.html @@ -27,6 +27,31 @@ {% endmacro %} +{% macro renderExports(items, cssClass, itemType) %} +
+

{$ itemType $}

+ + + + + + {% for item in items %} + + + + + {% endfor %} + +
NameDescription
+ + {$ item.name | escape $} + + + {$ item.shortDescription | marked $} +
+
+{% endmacro %} + {% block overview %} {% include "includes/class-overview.html" %} {% endblock %} @@ -47,8 +72,11 @@ {$ renderTable(doc.ngmoduleOptions.providers, 'providers', 'Providers', 'Provider') $} {% endif %} - {% if doc.ngmoduleOptions.exports %} - {$ renderTable(doc.ngmoduleOptions.exports, 'exports', 'Exports', 'Export') $} + {% if doc.directives.length %} + {$ renderExports(doc.directives, 'directive', 'Directives') $} + {% endif %} + {% if doc.pipes.length %} + {$ renderExports(doc.pipes, 'pipe', 'Pipes') $} {% endif %} {% endblock %} diff --git a/aio/tools/transforms/templates/api/package.template.html b/aio/tools/transforms/templates/api/package.template.html index f6bd146637..035415d27b 100644 --- a/aio/tools/transforms/templates/api/package.template.html +++ b/aio/tools/transforms/templates/api/package.template.html @@ -32,6 +32,7 @@ {% endif %}

{% if doc.isPrimaryPackage %}Primary entry{% else %}Entry{% endif %} point exports

+ {$ listItems(doc.ngmodules, 'NgModules') $} {$ listItems(doc.classes, 'Classes') $} {$ listItems(doc.decorators, 'Decorators') $} {$ listItems(doc.functions, 'Functions') $}