From f0366843ea157f17cfafcfc49a72c31d69561caa Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Wed, 16 Oct 2019 16:28:09 -0700 Subject: [PATCH] fix(bazel): Remove angular devkit and restore ngc postinstall (#32946) This commit removes `@angular-devkit/build-angular` from package.json for a project that opts into Bazel. This is because the package adds a dependency on node-sass, which is rejected by Bazel due to its absense. This commit also appends to `scripts.postinstall` if it already exists. This is needed because `ng new` in CLI v9 now automatically adds a postinstall step for `ngcc`. PR Close #32946 --- integration/bazel-schematics/test.sh | 10 +++- packages/bazel/src/schematics/ng-add/index.ts | 54 +++++++++++++++++-- .../bazel/src/schematics/ng-add/index_spec.ts | 23 ++++++++ 3 files changed, 82 insertions(+), 5 deletions(-) mode change 100755 => 100644 packages/bazel/src/schematics/ng-add/index.ts diff --git a/integration/bazel-schematics/test.sh b/integration/bazel-schematics/test.sh index 89ad68eb41..bfda881a92 100755 --- a/integration/bazel-schematics/test.sh +++ b/integration/bazel-schematics/test.sh @@ -21,7 +21,7 @@ function installLocalPackages() { local_packages+=("tslib@file:${pwd}/../../../node_modules/tslib") local_packages+=("@types/node@file:${pwd}/../../../node_modules/@types/node") - yarn add "${local_packages[@]}" + yarn add --ignore-scripts --silent "${local_packages[@]}" } @@ -50,13 +50,19 @@ function testNonBazel() { # Replace angular.json that uses Bazel builder with the default generated by CLI mv ./angular.json.bak ./angular.json rm -rf dist src/main.dev.ts src/main.prod.ts - yarn webdriver-manager update --gecko=false --standalone=false ${CI_CHROMEDRIVER_VERSION_ARG:---versions.chrome 2.45} # disable CLI's version check (if version is 0.0.0, then no version check happens) yarn --cwd node_modules/@angular/cli version --new-version 0.0.0 --no-git-tag-version + # re-add build-angular + yarn add --dev @angular-devkit/build-angular@0.900.0-next.11 + yarn webdriver-manager update --gecko=false --standalone=false ${CI_CHROMEDRIVER_VERSION_ARG:---versions.chrome 2.45} ng build --progress=false ng test --progress=false --watch=false ng e2e --configuration=production --webdriver-update=false } testBazel + +# this test verifies that users can undo bazel - the value of this is questionable +# because there are way too many manual steps and it would be easier for users to +# just revert the diff created by `ng add @angular/bazel` testNonBazel diff --git a/packages/bazel/src/schematics/ng-add/index.ts b/packages/bazel/src/schematics/ng-add/index.ts old mode 100755 new mode 100644 index d8e837a956..ffddb54893 --- a/packages/bazel/src/schematics/ng-add/index.ts +++ b/packages/bazel/src/schematics/ng-add/index.ts @@ -76,6 +76,47 @@ function addDevDependenciesToPackageJson(options: Schema) { }; } +/** + * Remove packages that are not needed under Bazel. + * @param options + */ +function removeObsoleteDependenciesFromPackageJson(options: Schema) { + return (host: Tree) => { + const packageJson = 'package.json'; + if (!host.exists(packageJson)) { + throw new Error(`Could not find ${packageJson}`); + } + const buffer = host.read(packageJson); + if (!buffer) { + throw new Error('Failed to read package.json content'); + } + const content = buffer.toString(); + const jsonAst = parseJsonAst(content) as JsonAstObject; + const deps = findPropertyInAstObject(jsonAst, 'dependencies') as JsonAstObject; + const devDeps = findPropertyInAstObject(jsonAst, 'devDependencies') as JsonAstObject; + + const depsToRemove = [ + '@angular-devkit/build-angular', + ]; + + const recorder = host.beginUpdate(packageJson); + + for (const packageName of depsToRemove) { + const depNode = findPropertyInAstObject(deps, packageName); + if (depNode) { + removeKeyValueInAstObject(recorder, content, deps, packageName); + } + const devDepNode = findPropertyInAstObject(devDeps, packageName); + if (devDepNode) { + removeKeyValueInAstObject(recorder, content, devDeps, packageName); + } + } + + host.commitUpdate(recorder); + return host; + }; +} + /** * Append additional Javascript / Typescript files needed to compile an Angular * project under Bazel. @@ -292,13 +333,19 @@ function addPostinstallToGenerateNgSummaries() { } const scripts = findPropertyInAstObject(jsonAst, 'scripts') as JsonAstObject; const recorder = host.beginUpdate(packageJson); + const ngcCommand = 'ngc -p ./angular-metadata.tsconfig.json'; if (scripts) { - insertPropertyInAstObjectInOrder( - recorder, scripts, 'postinstall', 'ngc -p ./angular-metadata.tsconfig.json', 4); + const postInstall = findPropertyInAstObject(scripts, 'postinstall'); + if (postInstall) { + const command = `${postInstall.value}; ${ngcCommand}`; + replacePropertyInAstObject(recorder, scripts, 'postinstall', command); + } else { + insertPropertyInAstObjectInOrder(recorder, scripts, 'postinstall', ngcCommand, 4); + } } else { insertPropertyInAstObjectInOrder( recorder, jsonAst, 'scripts', { - postinstall: 'ngc -p ./angular-metadata.tsconfig.json', + postinstall: ngcCommand, }, 2); } @@ -329,6 +376,7 @@ export default function(options: Schema): Rule { return chain([ addFilesRequiredByBazel(options), addDevDependenciesToPackageJson(options), + removeObsoleteDependenciesFromPackageJson(options), addPostinstallToGenerateNgSummaries(), backupAngularJson(), updateAngularJsonToUseBazelBuilder(options), diff --git a/packages/bazel/src/schematics/ng-add/index_spec.ts b/packages/bazel/src/schematics/ng-add/index_spec.ts index 5f1ccc588d..0ad9ebf1e4 100644 --- a/packages/bazel/src/schematics/ng-add/index_spec.ts +++ b/packages/bazel/src/schematics/ng-add/index_spec.ts @@ -145,6 +145,29 @@ describe('ng-add schematic', () => { expect(json.devDependencies['@angular/bazel']).toBe('1.2.3'); }); + it('should remove unneeded dependencies', async() => { + const packageJson = JSON.parse(host.readContent('/package.json')); + packageJson.devDependencies['@angular-devkit/build-angular'] = '1.2.3'; + host.overwrite('/package.json', JSON.stringify(packageJson)); + host = await schematicRunner.runSchematicAsync('ng-add', defaultOptions, host).toPromise(); + const content = host.readContent('/package.json'); + const json = JSON.parse(content); + expect(json.devDependencies['angular-devkit/build-angular']).toBeUndefined(); + }); + + it('should append to scripts.postinstall if it already exists', async() => { + const packageJson = JSON.parse(host.readContent('/package.json')); + packageJson['scripts'] = { + postinstall: 'angular rocks', + }; + host.overwrite('/package.json', JSON.stringify(packageJson)); + host = await schematicRunner.runSchematicAsync('ng-add', defaultOptions, host).toPromise(); + const content = host.readContent('/package.json'); + const json = JSON.parse(content); + expect(json.scripts['postinstall']) + .toBe('angular rocks; ngc -p ./angular-metadata.tsconfig.json'); + }); + it('should not create Bazel workspace file', async() => { host = await schematicRunner.runSchematicAsync('ng-add', defaultOptions, host).toPromise(); const {files} = host;