aio: autolinking fixes #20512

petebacondarwin · 2017-11-17T21:46:43Z

Closes #20466

mary-poppins · 2017-11-17T21:58:17Z

You can preview c4d5b29 at https://pr20512-c4d5b29.ngbuilds.io/.

gkalpak · 2017-11-20T11:43:59Z

aio/tools/transforms/angular-api-package/processors/addMetadataAliases.js

@@ -1,3 +1,5 @@
+const  CssSelectorParser = require('css-selector-parser').CssSelectorParser;


OMG, extra whitespace after const 😱

gkalpak · 2017-11-20T11:46:08Z

aio/tools/transforms/angular-api-package/processors/addMetadataAliases.js

+  const selectorAST = cssParser.parse(stripQuotes(selectors));
+  const rules = selectorAST.selectors ? selectorAST.selectors.map(ruleSet => ruleSet.rule) : [selectorAST.rule];
+  return rules.reduce((aliases, rule) => {
+    const tagAliases = rule.tagName? [rule.tagName] : [];


Missing whitespace before ? 😢

gkalpak · 2017-11-20T12:05:14Z

aio/tools/transforms/angular-base-package/services/filterAmbiguousDirectiveAliases.spec.js

+      expect(filterAmbiguousDirectiveAliases(docs, words, 2)).toEqual(filteredDocs);
+    });
+
+    it('should filter out docs that do not match (case-insensitively) their class name to the matching word', () => {


This description seems a little off.

gkalpak · 2017-11-20T12:06:28Z

aio/tools/transforms/angular-base-package/services/filterAmbiguousDirectiveAliases.js

+    if (docs.length > 1) {
+
+      // Capture the first doc's docType
+      const docType = docs[0].docType;


What is the reasoning for caring only about the first doc?

because we don't try to do any filtering if the docs are not all of the same type.

And what is the reasoning for not trying to do any filtering if the docs are not all of the same type? 😁

gkalpak · 2017-11-20T12:08:59Z

aio/tools/transforms/angular-base-package/services/filterAmbiguousDirectiveAliases.js

+
+        if (docs.every(doc =>
+          // if all the matched docs are of the same type (directives or components)
+          doc.docType === docType &&


So, if the first one is of type component we don't filter if any of the rest is of type directive?
Why is that?

In the core docs there are not really many components (if any) so I don't think that this really matters either way. I guess we can be more relaxed and allow any collection of directives and components (just nothing else).

gkalpak · 2017-11-20T12:10:29Z

aio/tools/transforms/angular-base-package/services/filterAmbiguousDirectiveAliases.js

+          // if all the matched docs are of the same type (directives or components)
+          doc.docType === docType &&
+          // and the matching word is in the selector for all of them
+          doc[docType + 'Options'].selector.indexOf(word) != -1


Should we take word boundaries into account. E.g. ['ngModelStatus'].indexOf('ngModel') !== -1, but should that count as a match?

I guess you are saying that you think it should not?

I wonder if this is likely to be a problem...?
For a doc to appear in the original docs list that is passed to the filter function, the matching word must appear as an alias of the doc. So I think that it is unlikely that the matching word does not appear in the selector at all.

…ponents

Having duplicates was causing the code auto-linking to ignore `ngForm` directives.

Closes angular#20466

mary-poppins · 2017-11-20T15:02:41Z

You can preview dad6f35 at https://pr20512-dad6f35.ngbuilds.io/.

gkalpak · 2017-11-20T18:57:21Z

The Travis failures are unrelated to the PR.

IgorMinar · 2017-11-21T20:18:25Z

@petebacondarwin please add target label in the future. in general for docs, bug fixes and infrastructure changes we should always try to cherrypick the commit to the stable branch

gkalpak · 2017-11-21T20:39:24Z

Ooops! I added the merge label without checking for target 😳

petebacondarwin · 2017-11-22T08:08:06Z

@IgorMinar yes, I normally add the target to my own PRs when I add the merge label. I didn't realise that @gkalpak had added the merge label when he approved it.

Having duplicates was causing the code auto-linking to ignore `ngForm` directives. PR Close #20512

…20512) Closes #20466 PR Close #20512

…ponents (#20512) PR Close #20512

Having duplicates was causing the code auto-linking to ignore `ngForm` directives. PR Close #20512

…20512) Closes #20466 PR Close #20512

…ponents (angular#20512) PR Close angular#20512

Having duplicates was causing the code auto-linking to ignore `ngForm` directives. PR Close angular#20512

…ngular#20512) Closes angular#20466 PR Close angular#20512

angular-automatic-lock-bot · 2019-09-12T22:20:50Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

petebacondarwin added comp: aio action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 17, 2017

petebacondarwin force-pushed the aio-autolinking-fixes branch from e55c3dc to c4d5b29 Compare November 17, 2017 21:46

googlebot added the cla: yes label Nov 17, 2017

petebacondarwin added this to REVIEW in docs-infra Nov 17, 2017

petebacondarwin requested a review from gkalpak November 18, 2017 07:05

gkalpak reviewed Nov 20, 2017

View reviewed changes

petebacondarwin added 3 commits November 20, 2017 14:46

build(aio): better parsing of selectors as aliases for directives/com…

2d6b221

…ponents

build(aio): do not store duplicate metadata aliases

62817c7

Having duplicates was causing the code auto-linking to ignore `ngForm` directives.

build(aio): filter out ambiguous directives from auto code linking

dad6f35

Closes angular#20466

petebacondarwin force-pushed the aio-autolinking-fixes branch from c4d5b29 to dad6f35 Compare November 20, 2017 14:47

gkalpak approved these changes Nov 20, 2017

View reviewed changes

gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 20, 2017

gkalpak moved this from REVIEW to MERGE in docs-infra Nov 20, 2017

IgorMinar added the target: patch This PR is targeted for the next patch release label Nov 21, 2017

mhevery pushed a commit that referenced this pull request Nov 22, 2017

build(aio): do not store duplicate metadata aliases (#20512)

ae0275e

Having duplicates was causing the code auto-linking to ignore `ngForm` directives. PR Close #20512

mhevery pushed a commit that referenced this pull request Nov 22, 2017

build(aio): filter out ambiguous directives from auto code linking (#…

e2b76bb

…20512) Closes #20466 PR Close #20512

mhevery closed this in ecce907 Nov 22, 2017

mhevery pushed a commit that referenced this pull request Nov 22, 2017

build(aio): better parsing of selectors as aliases for directives/com…

e4bb077

…ponents (#20512) PR Close #20512

mhevery pushed a commit that referenced this pull request Nov 22, 2017

build(aio): do not store duplicate metadata aliases (#20512)

07699cb

Having duplicates was causing the code auto-linking to ignore `ngForm` directives. PR Close #20512

mhevery pushed a commit that referenced this pull request Nov 22, 2017

build(aio): filter out ambiguous directives from auto code linking (#…

0550383

…20512) Closes #20466 PR Close #20512

petebacondarwin removed this from MERGE in docs-infra Nov 22, 2017

petebacondarwin deleted the aio-autolinking-fixes branch November 22, 2017 17:12

wKoza pushed a commit to wKoza/angular that referenced this pull request Dec 2, 2017

build(aio): better parsing of selectors as aliases for directives/com…

0643297

…ponents (angular#20512) PR Close angular#20512

wKoza pushed a commit to wKoza/angular that referenced this pull request Dec 2, 2017

build(aio): do not store duplicate metadata aliases (angular#20512)

7144d87

Having duplicates was causing the code auto-linking to ignore `ngForm` directives. PR Close angular#20512

wKoza pushed a commit to wKoza/angular that referenced this pull request Dec 2, 2017

build(aio): filter out ambiguous directives from auto code linking (a…

d4f11cc

…ngular#20512) Closes angular#20466 PR Close angular#20512

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aio: autolinking fixes #20512

aio: autolinking fixes #20512

petebacondarwin commented Nov 17, 2017

mary-poppins commented Nov 17, 2017

gkalpak Nov 20, 2017

gkalpak Nov 20, 2017

gkalpak Nov 20, 2017

gkalpak Nov 20, 2017

petebacondarwin Nov 20, 2017

gkalpak Nov 20, 2017

gkalpak Nov 20, 2017

petebacondarwin Nov 20, 2017

gkalpak Nov 20, 2017

petebacondarwin Nov 20, 2017

petebacondarwin Nov 20, 2017

mary-poppins commented Nov 20, 2017

gkalpak commented Nov 20, 2017

IgorMinar commented Nov 21, 2017

gkalpak commented Nov 21, 2017

petebacondarwin commented Nov 22, 2017

angular-automatic-lock-bot bot commented Sep 12, 2019

		@@ -1,3 +1,5 @@
		const CssSelectorParser = require('css-selector-parser').CssSelectorParser;

aio: autolinking fixes #20512

aio: autolinking fixes #20512

Conversation

petebacondarwin commented Nov 17, 2017

mary-poppins commented Nov 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mary-poppins commented Nov 20, 2017

gkalpak commented Nov 20, 2017

IgorMinar commented Nov 21, 2017

gkalpak commented Nov 21, 2017

petebacondarwin commented Nov 22, 2017

angular-automatic-lock-bot bot commented Sep 12, 2019