Skip to content

Commit 3ee9b47

Browse files
devversionandrewseguin
authored andcommitted
refactor(schematics): do not depend on parse5 twice (#13594)
* 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.
1 parent 1ccd1ee commit 3ee9b47

File tree

6 files changed

+20
-30
lines changed

6 files changed

+20
-30
lines changed

src/lib/schematics/ng-add/fonts/head-element.ts renamed to src/cdk/schematics/utils/html-head-element.ts

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,32 +6,28 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {WorkspaceProject} from '@angular-devkit/core/src/workspace';
109
import {SchematicsException, Tree} from '@angular-devkit/schematics';
11-
import {getChildElementIndentation} from '@angular/cdk/schematics';
10+
import {getChildElementIndentation} from './parse5-element';
1211
import {DefaultTreeDocument, DefaultTreeElement, parse as parseHtml} from 'parse5';
13-
import {getIndexHtmlPath} from './project-index-html';
1412

13+
/** Appends the given element HTML fragment to the `<head>` element of the specified HTML file. */
14+
export function appendHtmlElementToHead(host: Tree, htmlFilePath: string, elementHtml: string) {
15+
const htmlFileBuffer = host.read(htmlFilePath);
1516

16-
/** Appends the given element HTML fragment to the index.html head tag. */
17-
export function appendElementToHead(host: Tree, project: WorkspaceProject, elementHtml: string) {
18-
const indexPath = getIndexHtmlPath(project);
19-
const indexHtmlBuffer = host.read(indexPath);
20-
21-
if (!indexHtmlBuffer) {
22-
throw new SchematicsException(`Could not find file for path: ${indexPath}`);
17+
if (!htmlFileBuffer) {
18+
throw new SchematicsException(`Could not read file for path: ${htmlFilePath}`);
2319
}
2420

25-
const htmlContent = indexHtmlBuffer.toString();
21+
const htmlContent = htmlFileBuffer.toString();
2622

2723
if (htmlContent.includes(elementHtml)) {
2824
return;
2925
}
3026

31-
const headTag = getHeadTagElement(htmlContent);
27+
const headTag = getHtmlHeadTagElement(htmlContent);
3228

3329
if (!headTag) {
34-
throw `Could not find '<head>' element in HTML file: ${indexPath}`;
30+
throw `Could not find '<head>' element in HTML file: ${htmlFileBuffer}`;
3531
}
3632

3733
// 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
4137
const insertion = `${' '.repeat(indentationOffset)}${elementHtml}`;
4238

4339
const recordedChange = host
44-
.beginUpdate(indexPath)
40+
.beginUpdate(htmlFilePath)
4541
.insertRight(endTagOffset, `${insertion}\n`);
4642

4743
host.commitUpdate(recordedChange);
4844
}
4945

5046
/** Parses the given HTML file and returns the head element if available. */
51-
export function getHeadTagElement(src: string): DefaultTreeElement | null {
52-
const document = parseHtml(src, {sourceCodeLocationInfo: true}) as DefaultTreeDocument;
47+
export function getHtmlHeadTagElement(htmlContent: string): DefaultTreeElement | null {
48+
const document = parseHtml(htmlContent, {sourceCodeLocationInfo: true}) as DefaultTreeDocument;
5349
const nodeQueue = [...document.childNodes];
5450

5551
while (nodeQueue.length) {

src/cdk/schematics/utils/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export * from './ast';
1010
export * from './ast/ng-module-imports';
1111
export * from './build-component';
1212
export * from './get-project';
13+
export * from './html-head-element';
1314
export * from './parse5-element';
1415
export * from './project-main-file';
1516
export * from './project-style-file';

src/lib/package.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,6 @@
3030
"dependencies": {
3131
"tslib": "^1.7.1"
3232
},
33-
"optionalDependencies": {
34-
"parse5": "^5.0.0"
35-
},
3633
"schematics": "./schematics/collection.json",
3734
"ng-update": {
3835
"migrations": "./schematics/migration.json",

src/lib/schematics/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ ts_library(
2525
# TODO(devversion): Only include jasmine for test sources (See: tsconfig types).
2626
"@npm//@types/jasmine",
2727
"@npm//@types/node",
28-
"@npm//parse5",
2928
"@npm//tslint",
3029
"@npm//typescript",
3130
],

src/lib/schematics/ng-add/fonts/material-fonts.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,26 @@
77
*/
88

99
import {Tree} from '@angular-devkit/schematics';
10-
import {getProjectFromWorkspace} from '@angular/cdk/schematics';
10+
import {appendHtmlElementToHead, getProjectFromWorkspace} from '@angular/cdk/schematics';
1111
import {getWorkspace} from '@schematics/angular/utility/config';
1212
import {Schema} from '../schema';
13-
import {appendElementToHead} from './head-element';
13+
import {getIndexHtmlPath} from './project-index-html';
1414

1515
/** Adds the Material Design fonts to the index HTML file. */
1616
export function addFontsToIndex(options: Schema): (host: Tree) => Tree {
1717
return (host: Tree) => {
1818
const workspace = getWorkspace(host);
1919
const project = getProjectFromWorkspace(workspace, options.project);
20+
const projectIndexHtmlPath = getIndexHtmlPath(project);
2021

2122
const fonts = [
2223
'https://fonts.googleapis.com/css?family=Roboto:300,400,500',
2324
'https://fonts.googleapis.com/icon?family=Material+Icons',
2425
];
2526

26-
fonts.forEach(f => appendElementToHead(host, project, `<link href="${f}" rel="stylesheet">`));
27+
fonts.forEach(f => {
28+
appendHtmlElementToHead(host, projectIndexHtmlPath, `<link href="${f}" rel="stylesheet">`);
29+
});
2730

2831
return host;
2932
};

src/lib/schematics/ng-add/setup-project.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {chain, noop, Rule, SchematicsException, Tree} from '@angular-devkit/schematics';
9+
import {chain, noop, Rule, Tree} from '@angular-devkit/schematics';
1010
import {
1111
addModuleImportToRootModule,
1212
getProjectFromWorkspace,
@@ -17,7 +17,6 @@ import {
1717
import {red, bold} from 'chalk';
1818
import {getWorkspace} from '@schematics/angular/utility/config';
1919
import {getAppModulePath} from '@schematics/angular/utility/ng-ast-utils';
20-
import * as parse5 from 'parse5';
2120
import {addFontsToIndex} from './fonts/material-fonts';
2221
import {addHammerJsToMain} from './gestures/hammerjs-import';
2322
import {Schema} from './schema';
@@ -36,11 +35,6 @@ const noopAnimationsModuleName = 'NoopAnimationsModule';
3635
* - Adds Browser Animation to app.module
3736
*/
3837
export default function(options: Schema): Rule {
39-
if (!parse5) {
40-
throw new SchematicsException('Parse5 is required but could not be found! Please install ' +
41-
'"parse5" manually in order to continue.');
42-
}
43-
4438
return chain([
4539
options && options.gestures ? addHammerJsToMain(options) : noop(),
4640
addAnimationsModule(options),

0 commit comments

Comments
 (0)