From c6b180380a3b49449a4de990daa1c2b38a12766b Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Fri, 14 Jun 2019 14:02:56 +0300 Subject: [PATCH] fix(docs-infra): do not auto-link `http://` to the `common/http` (#31051) Previously, our auto-linking feature would match `http` in URLs (such as `http://...`) to the `common/http` package and automatically create a link to that, which was undesirable. While it is possible to work around that via `http://...`, most people didn't even realize the issue. Since in this case it is possible to reliably know it is a false match, this commit fixes it by applying a custom auto-link filter that ignores all docs for `http`, if it comes before `://`. Fixes #31012 PR Close #31051 --- .../transforms/angular-base-package/index.js | 5 +- .../post-processors/auto-link-code.js | 3 +- .../services/filterPipes.js | 2 +- .../services/ignoreHttpInUrls.js | 12 +++++ .../services/ignoreHttpInUrls.spec.js | 49 +++++++++++++++++++ 5 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 aio/tools/transforms/angular-base-package/services/ignoreHttpInUrls.js create mode 100644 aio/tools/transforms/angular-base-package/services/ignoreHttpInUrls.spec.js diff --git a/aio/tools/transforms/angular-base-package/index.js b/aio/tools/transforms/angular-base-package/index.js index 2c5a679cba..ab0a63518e 100644 --- a/aio/tools/transforms/angular-base-package/index.js +++ b/aio/tools/transforms/angular-base-package/index.js @@ -38,6 +38,7 @@ module.exports = new Package('angular-base', [ .factory(require('./services/copyFolder')) .factory(require('./services/filterPipes')) .factory(require('./services/filterAmbiguousDirectiveAliases')) + .factory(require('./services/ignoreHttpInUrls')) .factory(require('./services/getImageDimensions')) .factory(require('./post-processors/add-image-dimensions')) @@ -128,9 +129,9 @@ module.exports = new Package('angular-base', [ }) - .config(function(postProcessHtml, addImageDimensions, autoLinkCode, filterPipes, filterAmbiguousDirectiveAliases) { + .config(function(postProcessHtml, addImageDimensions, autoLinkCode, filterPipes, filterAmbiguousDirectiveAliases, ignoreHttpInUrls) { addImageDimensions.basePath = path.resolve(AIO_PATH, 'src'); - autoLinkCode.customFilters = [filterPipes, filterAmbiguousDirectiveAliases]; + autoLinkCode.customFilters = [ignoreHttpInUrls, filterPipes, filterAmbiguousDirectiveAliases]; postProcessHtml.plugins = [ require('./post-processors/autolink-headings'), addImageDimensions, diff --git a/aio/tools/transforms/angular-base-package/post-processors/auto-link-code.js b/aio/tools/transforms/angular-base-package/post-processors/auto-link-code.js index 5285de6e62..2f562cccf0 100644 --- a/aio/tools/transforms/angular-base-package/post-processors/auto-link-code.js +++ b/aio/tools/transforms/angular-base-package/post-processors/auto-link-code.js @@ -38,7 +38,7 @@ module.exports = function autoLinkCode(getDocFromAlias) { // Only interested in text nodes that are not inside links if (ancestors.every(ancestor => !is(ancestor, 'a'))) { - const parent = ancestors[ancestors.length-1]; + const parent = ancestors[ancestors.length - 1]; const index = parent.children.indexOf(node); // Can we convert the whole text node into a doc link? @@ -66,6 +66,7 @@ module.exports = function autoLinkCode(getDocFromAlias) { }); }; } + function foundValidDoc(docs) { return docs.length === 1 && !docs[0].internal && diff --git a/aio/tools/transforms/angular-base-package/services/filterPipes.js b/aio/tools/transforms/angular-base-package/services/filterPipes.js index f6e334c20f..ebaf0c5764 100644 --- a/aio/tools/transforms/angular-base-package/services/filterPipes.js +++ b/aio/tools/transforms/angular-base-package/services/filterPipes.js @@ -1,6 +1,6 @@ /** - * This service is used by the autoLinkCode post-processors to filter out pipe docs + * This service is used by the autoLinkCode post-processor to filter out pipe docs * where the matching word is the pipe name and is not preceded by a pipe */ module.exports = function filterPipes() { diff --git a/aio/tools/transforms/angular-base-package/services/ignoreHttpInUrls.js b/aio/tools/transforms/angular-base-package/services/ignoreHttpInUrls.js new file mode 100644 index 0000000000..78b562499b --- /dev/null +++ b/aio/tools/transforms/angular-base-package/services/ignoreHttpInUrls.js @@ -0,0 +1,12 @@ + +/** + * This service is used by the autoLinkCode post-processor to ignore the `http(s)` part in URLs + * (i.e. `http://...` or `https://...`). + */ +module.exports = function ignoreHttpInUrls() { + const ignoredSchemes = ['http', 'https']; + return (docs, words, index) => { + const httpInUrl = ignoredSchemes.includes(words[index]) && (words[index + 1] === '://'); + return httpInUrl ? [] : docs; + }; +}; diff --git a/aio/tools/transforms/angular-base-package/services/ignoreHttpInUrls.spec.js b/aio/tools/transforms/angular-base-package/services/ignoreHttpInUrls.spec.js new file mode 100644 index 0000000000..37b31304e4 --- /dev/null +++ b/aio/tools/transforms/angular-base-package/services/ignoreHttpInUrls.spec.js @@ -0,0 +1,49 @@ +const ignoreHttpInUrls = require('./ignoreHttpInUrls')(); + +describe('ignoreHttpInUrls', () => { + it('should ignore all docs when matching `http` in `http://...`', () => { + const docs = [{ docType: 'package', name: 'http' }, { docType: 'class', name: 'Foo' }]; + + const words1 = ['http', '://', 'example', '.', 'com', '/']; + expect(ignoreHttpInUrls(docs, words1, 0)).toEqual([]); + + const words2 = ['URL', ': ', 'http', '://', 'example', '.', 'com', '/']; + expect(ignoreHttpInUrls(docs, words2, 2)).toEqual([]); + }); + + it('should ignore all docs when matching `https` in `https://...`', () => { + const docs = [{ docType: 'package', name: 'https' }, { docType: 'class', name: 'Foo' }]; + + const words1 = ['https', '://', 'example', '.', 'com', '/']; + expect(ignoreHttpInUrls(docs, words1, 0)).toEqual([]); + + const words2 = ['URL', ': ', 'https', '://', 'example', '.', 'com', '/']; + expect(ignoreHttpInUrls(docs, words2, 2)).toEqual([]); + }); + + it('should keep all docs when not matching `http(s)`', () => { + const docs = [{ docType: 'package', name: 'http' }, { docType: 'class', name: 'Foo' }]; + + const words1 = ['http', '://', 'example', '.', 'com', '/']; + expect(ignoreHttpInUrls(docs, words1, 2)).toEqual(docs); + + const words2 = ['URL', ': ', 'https', '://', 'example', '.', 'com', '/']; + expect(ignoreHttpInUrls(docs, words2, 0)).toEqual(docs); + + const words3 = ['file', '://', 'example', '.', 'com', '/']; + expect(ignoreHttpInUrls(docs, words3, 0)).toEqual(docs); + }); + + it('should keep all docs when not matching `http(s)` at the beginning of a URL', () => { + const docs = [{ docType: 'package', name: 'http' }, { docType: 'class', name: 'Foo' }]; + + const words1 = ['http', ' ', 'is', ' ', 'cool']; + expect(ignoreHttpInUrls(docs, words1, 0)).toEqual(docs); + + const words2 = ['https', ' ', 'is', ' ', 'cooler']; + expect(ignoreHttpInUrls(docs, words2, 0)).toEqual(docs); + + const words3 = ['http', '://', 'http', '.', 'com']; + expect(ignoreHttpInUrls(docs, words3, 2)).toEqual(docs); + }); +});