build(aio): ensure the correct decorator properties are merged (#24289)

Previously only the `description` and `usageNotes` were being copied over
from the call-member of the decorator interface. Important properties such
as `shortDescription` were missed.

These are now added and the code has been refactored to make it simpler and
clearer to update which properties get copied as the requirements change.

PR Close #24289
This commit is contained in:
Pete Bacon Darwin 2018-06-04 11:43:15 +01:00 committed by Victor Berchet
parent acf270d724
commit 68d37ef0c1
5 changed files with 79 additions and 13 deletions

View File

@ -119,6 +119,17 @@ module.exports = new Package('angular-api', [basePackage, typeScriptPackage])
addNotYetDocumentedProperty.docTypes = API_DOC_TYPES;
})
.config(function(mergeDecoratorDocs) {
mergeDecoratorDocs.propertiesToMerge = [
'shortDescription',
'description',
'security',
'deprecated',
'see',
'usageNotes',
];
})
.config(function(checkContentRules, EXPORT_DOC_TYPES) {
// Min length rules
const createMinLengthRule = require('./content-rules/minLength');

View File

@ -1,3 +1,5 @@
const {mergeProperties} = require('../../helpers/utils');
/**
* Decorators in the Angular code base are made up from three code items:
*
@ -47,6 +49,7 @@ module.exports = function mergeDecoratorDocs(log) {
return {
$runAfter: ['processing-docs'],
$runBefore: ['docs-processed'],
propertiesToMerge: [],
makeDecoratorCalls: [
{type: '', description: 'toplevel', functionName: 'makeDecorator'},
{type: 'Prop', description: 'property', functionName: 'makePropDecorator'},
@ -55,6 +58,7 @@ module.exports = function mergeDecoratorDocs(log) {
$process: function(docs) {
var makeDecoratorCalls = this.makeDecoratorCalls;
var propertiesToMerge = this.propertiesToMerge;
var docsToMerge = Object.create(null);
docs.forEach(function(doc) {
@ -88,9 +92,9 @@ module.exports = function mergeDecoratorDocs(log) {
log.debug(
'mergeDecoratorDocs: merging', doc.name, 'into', decoratorDoc.name,
callMember.description.substring(0, 50));
// Merge the documentation found in this call signature into the original decorator
decoratorDoc.description = callMember.description;
decoratorDoc.usageNotes = callMember.usageNotes;
mergeProperties(decoratorDoc, callMember, propertiesToMerge);
// remove doc from its module doc's exports
doc.moduleDoc.exports =

View File

@ -9,17 +9,21 @@ describe('mergeDecoratorDocs processor', () => {
const injector = dgeni.configureInjector();
processor = injector.get('mergeDecoratorDocs');
// Note that we do not include usageNotes in the tests.
processor.propertiesToMerge = ['description', 'shortDescription'];
moduleDoc = {};
decoratorDoc = {
name: 'Component',
docType: 'const',
description: 'A description of the metadata for the Component decorator',
shortDescription: 'decorator - short description',
description: 'decorator - description',
symbol: {
valueDeclaration: { initializer: { expression: { text: 'makeDecorator' }, arguments: [{ text: 'X' }] } }
},
members: [
{ name: 'templateUrl', description: 'A description of the templateUrl property' }
{ name: 'templateUrl', description: 'templateUrl - description' }
],
moduleDoc
};
@ -27,15 +31,15 @@ describe('mergeDecoratorDocs processor', () => {
metadataDoc = {
name: 'ComponentDecorator',
docType: 'interface',
description: 'A description of the interface for the call signature for the Component decorator',
description: 'call interface - description',
members: [
{
isCallMember: true,
description: 'The actual description of the call signature',
usageNotes: 'Use it like this...'
description: 'call interface - call member - description',
usageNotes: 'call interface - call member - usageNotes',
},
{
description: 'Some other member'
description: 'call interface - non call member - description'
}
],
moduleDoc
@ -65,10 +69,13 @@ describe('mergeDecoratorDocs processor', () => {
expect(decoratorDoc.decoratorType).toEqual('X');
});
it('should copy across properties from the call signature doc', () => {
it('should copy across specified properties from the call signature doc', () => {
processor.$process([decoratorDoc, metadataDoc, otherDoc]);
expect(decoratorDoc.description).toEqual('The actual description of the call signature');
expect(decoratorDoc.usageNotes).toEqual('Use it like this...');
expect(decoratorDoc.description).toEqual('call interface - call member - description');
// Since usageNotes is not in `propertiesToMerge` it will not get copied over in these tests.
expect(decoratorDoc.usageNotes).toBeUndefined();
// Since `shortDescription` does not exist on the call-member this will not get overridden.
expect(decoratorDoc.shortDescription).toEqual('decorator - short description');
});
it('should remove the metadataDoc from the module exports', () => {

View File

@ -96,5 +96,19 @@ module.exports = {
attrMap[key] === false ? '' :
attrMap[key] === true ? ` ${key}` :
` ${key}="${attrMap[key].replace(/"/g, '"')}"`).join('');
},
/**
* Merge the specified properties from the source to the target document
* @param {Document} target The document to receive the properties from the source
* @param {Document} source The document from which to get the properties to merge
* @param {string[]} properties A collection of the names of the properties to merge
*/
mergeProperties(target, source, properties) {
properties.forEach(property => {
if (source.hasOwnProperty(property)) {
target[property] = source[property];
}
});
},
};

View File

@ -1,4 +1,4 @@
const { mapObject, parseAttributes, renderAttributes } = require('./utils');
const { mergeProperties, mapObject, parseAttributes, renderAttributes } = require('./utils');
describe('utils', () => {
describe('mapObject', () => {
@ -96,4 +96,34 @@ describe('utils', () => {
expect(renderAttributes({ })).toEqual('');
});
});
describe('mergeProperties', () => {
it('should write specified properties from the source to the target', () => {
const source = { a: 1, b: 2, c: 3 };
const target = { };
mergeProperties(target, source, ['a', 'b']);
expect(target).toEqual({ a: 1, b: 2 });
});
it('should not overwrite target properties that are not specified', () => {
const source = { a: 1, b: 2, c: 3 };
const target = { b: 10 };
mergeProperties(target, source, ['a']);
expect(target).toEqual({ a: 1, b: 10 });
});
it('should not overwrite target properties that are specified but do not exist in the source', () => {
const source = { a: 1 };
const target = { b: 10 };
mergeProperties(target, source, ['a', 'b']);
expect(target).toEqual({ a: 1, b: 10 });
});
it('should overwrite target properties even if they are `undefined` in the source', () => {
const source = { a: undefined };
const target = { a: 10 };
mergeProperties(target, source, ['a']);
expect(target).toEqual({ a: undefined });
});
});
});