From c4c079282f2a65857351525410d307f446fc75e8 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 12 Oct 2018 21:35:25 +0200 Subject: [PATCH] refactor(schematics): do not depend on parse5 twice * As we've already moved the ng-update foundation to the CDK, and therefore introduced an `optionalDependency` on `parse5` for the CDK, we should also move the last `parse5` call from `@angular/material` to the `@angular/cdk`. This way we don't depend on `parse5` for both packages + we can re-use the abstract utility function within the CDK schematics if we need them. * The `parse5` runtime check is actually not working because NodeJS would throw already if we just `require("parse5")`. Since we marked `parse5` as a dependency and the default NodeJS `module not found` message is clear enough, we can remove this check from the `ng-add` command as well. --- .../schematics/utils/html-head-element.ts} | 28 ++++++++----------- src/cdk/schematics/utils/index.ts | 1 + src/lib/package.json | 3 -- src/lib/schematics/BUILD.bazel | 1 - .../schematics/ng-add/fonts/material-fonts.ts | 9 ++++-- src/lib/schematics/ng-add/setup-project.ts | 8 +----- 6 files changed, 20 insertions(+), 30 deletions(-) rename src/{lib/schematics/ng-add/fonts/head-element.ts => cdk/schematics/utils/html-head-element.ts} (58%) diff --git a/src/lib/schematics/ng-add/fonts/head-element.ts b/src/cdk/schematics/utils/html-head-element.ts similarity index 58% rename from src/lib/schematics/ng-add/fonts/head-element.ts rename to src/cdk/schematics/utils/html-head-element.ts index 28a32e1434aa..689ed73f7ce1 100644 --- a/src/lib/schematics/ng-add/fonts/head-element.ts +++ b/src/cdk/schematics/utils/html-head-element.ts @@ -6,32 +6,28 @@ * found in the LICENSE file at https://angular.io/license */ -import {WorkspaceProject} from '@angular-devkit/core/src/workspace'; import {SchematicsException, Tree} from '@angular-devkit/schematics'; -import {getChildElementIndentation} from '@angular/cdk/schematics'; +import {getChildElementIndentation} from './parse5-element'; import {DefaultTreeDocument, DefaultTreeElement, parse as parseHtml} from 'parse5'; -import {getIndexHtmlPath} from './project-index-html'; +/** Appends the given element HTML fragment to the `` element of the specified HTML file. */ +export function appendHtmlElementToHead(host: Tree, htmlFilePath: string, elementHtml: string) { + const htmlFileBuffer = host.read(htmlFilePath); -/** Appends the given element HTML fragment to the index.html head tag. */ -export function appendElementToHead(host: Tree, project: WorkspaceProject, elementHtml: string) { - const indexPath = getIndexHtmlPath(project); - const indexHtmlBuffer = host.read(indexPath); - - if (!indexHtmlBuffer) { - throw new SchematicsException(`Could not find file for path: ${indexPath}`); + if (!htmlFileBuffer) { + throw new SchematicsException(`Could not read file for path: ${htmlFilePath}`); } - const htmlContent = indexHtmlBuffer.toString(); + const htmlContent = htmlFileBuffer.toString(); if (htmlContent.includes(elementHtml)) { return; } - const headTag = getHeadTagElement(htmlContent); + const headTag = getHtmlHeadTagElement(htmlContent); if (!headTag) { - throw `Could not find '' element in HTML file: ${indexPath}`; + throw `Could not find '' element in HTML file: ${htmlFileBuffer}`; } // We always have access to the source code location here because the `getHeadTagElement` @@ -41,15 +37,15 @@ export function appendElementToHead(host: Tree, project: WorkspaceProject, eleme const insertion = `${' '.repeat(indentationOffset)}${elementHtml}`; const recordedChange = host - .beginUpdate(indexPath) + .beginUpdate(htmlFilePath) .insertRight(endTagOffset, `${insertion}\n`); host.commitUpdate(recordedChange); } /** Parses the given HTML file and returns the head element if available. */ -export function getHeadTagElement(src: string): DefaultTreeElement | null { - const document = parseHtml(src, {sourceCodeLocationInfo: true}) as DefaultTreeDocument; +export function getHtmlHeadTagElement(htmlContent: string): DefaultTreeElement | null { + const document = parseHtml(htmlContent, {sourceCodeLocationInfo: true}) as DefaultTreeDocument; const nodeQueue = [...document.childNodes]; while (nodeQueue.length) { diff --git a/src/cdk/schematics/utils/index.ts b/src/cdk/schematics/utils/index.ts index ab3e2b8c8683..8534f8214862 100644 --- a/src/cdk/schematics/utils/index.ts +++ b/src/cdk/schematics/utils/index.ts @@ -10,6 +10,7 @@ export * from './ast'; export * from './ast/ng-module-imports'; export * from './build-component'; export * from './get-project'; +export * from './html-head-element'; export * from './parse5-element'; export * from './project-main-file'; export * from './project-style-file'; diff --git a/src/lib/package.json b/src/lib/package.json index 6e54c9dbe617..65b7df0b9ad3 100644 --- a/src/lib/package.json +++ b/src/lib/package.json @@ -30,9 +30,6 @@ "dependencies": { "tslib": "^1.7.1" }, - "optionalDependencies": { - "parse5": "^5.0.0" - }, "schematics": "./schematics/collection.json", "ng-update": { "migrations": "./schematics/migration.json", diff --git a/src/lib/schematics/BUILD.bazel b/src/lib/schematics/BUILD.bazel index f9953c281e97..718dcbdc73d6 100644 --- a/src/lib/schematics/BUILD.bazel +++ b/src/lib/schematics/BUILD.bazel @@ -25,7 +25,6 @@ ts_library( # TODO(devversion): Only include jasmine for test sources (See: tsconfig types). "@npm//@types/jasmine", "@npm//@types/node", - "@npm//parse5", "@npm//tslint", "@npm//typescript", ], diff --git a/src/lib/schematics/ng-add/fonts/material-fonts.ts b/src/lib/schematics/ng-add/fonts/material-fonts.ts index d05b06ba3048..213ddc911b8a 100644 --- a/src/lib/schematics/ng-add/fonts/material-fonts.ts +++ b/src/lib/schematics/ng-add/fonts/material-fonts.ts @@ -7,23 +7,26 @@ */ import {Tree} from '@angular-devkit/schematics'; -import {getProjectFromWorkspace} from '@angular/cdk/schematics'; +import {appendHtmlElementToHead, getProjectFromWorkspace} from '@angular/cdk/schematics'; import {getWorkspace} from '@schematics/angular/utility/config'; import {Schema} from '../schema'; -import {appendElementToHead} from './head-element'; +import {getIndexHtmlPath} from './project-index-html'; /** Adds the Material Design fonts to the index HTML file. */ export function addFontsToIndex(options: Schema): (host: Tree) => Tree { return (host: Tree) => { const workspace = getWorkspace(host); const project = getProjectFromWorkspace(workspace, options.project); + const projectIndexHtmlPath = getIndexHtmlPath(project); const fonts = [ 'https://fonts.googleapis.com/css?family=Roboto:300,400,500', 'https://fonts.googleapis.com/icon?family=Material+Icons', ]; - fonts.forEach(f => appendElementToHead(host, project, ``)); + fonts.forEach(f => { + appendHtmlElementToHead(host, projectIndexHtmlPath, ``); + }); return host; }; diff --git a/src/lib/schematics/ng-add/setup-project.ts b/src/lib/schematics/ng-add/setup-project.ts index debf536417ee..bf47c5120f8b 100644 --- a/src/lib/schematics/ng-add/setup-project.ts +++ b/src/lib/schematics/ng-add/setup-project.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {chain, noop, Rule, SchematicsException, Tree} from '@angular-devkit/schematics'; +import {chain, noop, Rule, Tree} from '@angular-devkit/schematics'; import { addModuleImportToRootModule, getProjectFromWorkspace, @@ -17,7 +17,6 @@ import { import {red, bold} from 'chalk'; import {getWorkspace} from '@schematics/angular/utility/config'; import {getAppModulePath} from '@schematics/angular/utility/ng-ast-utils'; -import * as parse5 from 'parse5'; import {addFontsToIndex} from './fonts/material-fonts'; import {addHammerJsToMain} from './gestures/hammerjs-import'; import {Schema} from './schema'; @@ -36,11 +35,6 @@ const noopAnimationsModuleName = 'NoopAnimationsModule'; * - Adds Browser Animation to app.module */ export default function(options: Schema): Rule { - if (!parse5) { - throw new SchematicsException('Parse5 is required but could not be found! Please install ' + - '"parse5" manually in order to continue.'); - } - return chain([ options && options.gestures ? addHammerJsToMain(options) : noop(), addAnimationsModule(options),