Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aio: autolinking fixes #20512

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions aio/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
"codelyzer": "~2.0.0",
"concurrently": "^3.4.0",
"cross-spawn": "^5.1.0",
"css-selector-parser": "^1.3.0",
"dgeni": "^0.4.7",
"dgeni-packages": "0.22.0",
"entities": "^1.1.1",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const CssSelectorParser = require('css-selector-parser').CssSelectorParser;
const cssParser = new CssSelectorParser();
/**
* @dgProcessor addMetadataAliases
*
Expand Down Expand Up @@ -28,11 +30,18 @@ module.exports = function addMetadataAliasesProcessor() {
};

function extractSelectors(selectors) {
if (selectors) {
return stripQuotes(selectors).split(',').map(selector => selector.replace(/^\W*([\w-]+)\W*$/, '$1'));
} else {
return [];
}
const selectorAST = cssParser.parse(stripQuotes(selectors));
const rules = selectorAST.selectors ? selectorAST.selectors.map(ruleSet => ruleSet.rule) : [selectorAST.rule];
const aliases = {};
rules.forEach(rule => {
if (rule.tagName) {
aliases[rule.tagName] = true;
}
if (rule.attrs) {
rule.attrs.forEach(attr => aliases[attr.name] = true);
}
});
return Object.keys(aliases);
}

function stripQuotes(value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ describe('addSelectorsAsAliases processor', () => {
expect(docs[2].aliases).toEqual([docs[2].name]);
expect(docs[3].aliases).toEqual([docs[3].name]);
expect(docs[4].aliases).toEqual([docs[4].name, 'myPipe']);
expect(docs[5].aliases).toEqual([docs[5].name, 'my-directive', 'myDirective', 'my-directive']);
expect(docs[6].aliases).toEqual([docs[6].name, '[ngModel]:not([formControlName]):not([formControl])']);
expect(docs[5].aliases).toEqual([docs[5].name, 'my-directive', 'myDirective']);
expect(docs[6].aliases).toEqual([docs[6].name, 'ngModel']);
expect(docs[7].aliases).toEqual([docs[7].name, 'my-component']);
expect(docs[8].aliases).toEqual([docs[8].name]);
expect(docs[9].aliases).toEqual([docs[9].name]);
Expand Down
5 changes: 3 additions & 2 deletions aio/tools/transforms/angular-base-package/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ module.exports = new Package('angular-base', [
.factory(require('./readers/json'))
.factory(require('./services/copyFolder'))
.factory(require('./services/filterPipes'))
.factory(require('./services/filterAmbiguousDirectiveAliases'))
.factory(require('./services/getImageDimensions'))

.factory(require('./post-processors/add-image-dimensions'))
Expand Down Expand Up @@ -127,9 +128,9 @@ module.exports = new Package('angular-base', [
})


.config(function(postProcessHtml, addImageDimensions, autoLinkCode, filterPipes) {
.config(function(postProcessHtml, addImageDimensions, autoLinkCode, filterPipes, filterAmbiguousDirectiveAliases) {
addImageDimensions.basePath = path.resolve(AIO_PATH, 'src');
autoLinkCode.customFilters = [filterPipes];
autoLinkCode.customFilters = [filterPipes, filterAmbiguousDirectiveAliases];
postProcessHtml.plugins = [
require('./post-processors/autolink-headings'),
addImageDimensions,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@

/**
* This service is used by the autoLinkCode post-processor to filter out ambiguous directive
* docs where the matching word is a directive selector.
* E.g. `ngModel`, which is a selector for a number of directives, where we are only really
* interested in the `NgModel` class.
*/
module.exports = function filterAmbiguousDirectiveAliases() {
return (docs, words, index) => {
const word = words[index];

// we are only interested if there are multiple matching docs
if (docs.length > 1) {
if (docs.every(doc =>
// We are only interested if they are all either directives or components
(doc.docType === 'directive' || doc.docType === 'component') &&
// and the matching word is in the selector for all of them
doc[doc.docType + 'Options'].selector.indexOf(word) != -1
)) {
// find the directive whose class name matches the word (case-insensitive)
return docs.filter(doc => doc.name.toLowerCase() === word.toLowerCase());
}
}
return docs;
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
const filterAmbiguousDirectiveAliases = require('./filterAmbiguousDirectiveAliases')();

const words = ['Http', 'ngModel', 'NgModel', 'NgControlStatus'];

describe('filterAmbiguousDirectiveAliases(docs, words, index)', () => {
it('should not try to filter the docs, if the docs are not all directives or components', () => {
const docs = [
{ docType: 'class', name: 'Http' },
{ docType: 'directive', name: 'NgModel', directiveOptions: { selector: '[ngModel]' } },
{ docType: 'component', name: 'NgModel', componentOptions: { selector: '[ngModel]' } }
];
// take a copy to prove `docs` was not modified
const filteredDocs = docs.slice(0);
expect(filterAmbiguousDirectiveAliases(docs, words, 1)).toEqual(filteredDocs);
expect(filterAmbiguousDirectiveAliases(docs, words, 2)).toEqual(filteredDocs);
});

describe('(where all the docs are components or directives', () => {
describe('and do not all contain the matching word in their selector)', () => {
it('should not try to filter the docs', () => {
const docs = [
{ docType: 'directive', name: 'NgModel', ['directiveOptions']: { selector: '[ngModel]' } },
{ docType: 'component', name: 'NgControlStatus', ['componentOptions']: { selector: '[ngControlStatus]' } }
];
// take a copy to prove `docs` was not modified
const filteredDocs = docs.slice(0);
expect(filterAmbiguousDirectiveAliases(docs, words, 1)).toEqual(filteredDocs);
expect(filterAmbiguousDirectiveAliases(docs, words, 2)).toEqual(filteredDocs);

// Also test that the check is case-sensitive
docs[1].componentOptions.selector = '[ngModel]';
filteredDocs[1].componentOptions.selector = '[ngModel]';
expect(filterAmbiguousDirectiveAliases(docs, words, 2)).toEqual(filteredDocs);
});
});

describe('and do all contain the matching word in there selector)', () => {
it('should filter out docs whose class name is not (case-insensitively) equal to the matching word', () => {
const docs = [
{ docType: 'directive', name: 'NgModel', ['directiveOptions']: { selector: '[ngModel],[ngControlStatus]' } },
{ docType: 'component', name: 'NgControlStatus', ['componentOptions']: { selector: '[ngModel],[ngControlStatus]' } }
];
const filteredDocs = [
{ docType: 'directive', name: 'NgModel', ['directiveOptions']: { selector: '[ngModel],[ngControlStatus]' } }
];
expect(filterAmbiguousDirectiveAliases(docs, words, 1)).toEqual(filteredDocs);
});
});
});
});
4 changes: 4 additions & 0 deletions aio/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2007,6 +2007,10 @@ css-select@^1.1.0:
domutils "1.5.1"
nth-check "~1.0.1"

css-selector-parser@^1.3.0:
version "1.3.0"
resolved "https://registry.yarnpkg.com/css-selector-parser/-/css-selector-parser-1.3.0.tgz#5f1ad43e2d8eefbfdc304fcd39a521664943e3eb"

css-selector-tokenizer@^0.7.0:
version "0.7.0"
resolved "https://registry.yarnpkg.com/css-selector-tokenizer/-/css-selector-tokenizer-0.7.0.tgz#e6988474ae8c953477bf5e7efecfceccd9cf4c86"
Expand Down