From ca2c3e9fcc862c22cc98ea62ee06dfa3f24ba778 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 10 Nov 2020 12:27:30 -0500 Subject: [PATCH] fix(deploy): remove direct workspace access Depending on the configuration and initialization of the builder runtime, there may not be a workspace file or the raw values within the workspace file may not represent the final values used by a builder. To support cases where inspection of builder target options is needed, the builder runtime provides a utility function (`getTargetOptions`) within the context object passed to each builder. This utility function is now used within the deploy builder to access builder target options. Also, as a result of these changes, usage of the experimental workspace API was removed. This experimental API is no longer present as of Angular v11. --- src/schematics/deploy/actions.jasmine.ts | 47 +++++++++----------- src/schematics/deploy/actions.ts | 55 +++++++++++------------- src/schematics/deploy/builder.ts | 33 ++++---------- 3 files changed, 55 insertions(+), 80 deletions(-) diff --git a/src/schematics/deploy/actions.jasmine.ts b/src/schematics/deploy/actions.jasmine.ts index c723f07ab..c1a10175d 100644 --- a/src/schematics/deploy/actions.jasmine.ts +++ b/src/schematics/deploy/actions.jasmine.ts @@ -1,4 +1,4 @@ -import { experimental, JsonObject, logging } from '@angular-devkit/core'; +import { JsonObject, logging } from '@angular-devkit/core'; import { BuilderContext, BuilderRun, ScheduleOptions, Target } from '@angular-devkit/architect'; import { BuildTarget, FirebaseDeployConfig, FirebaseTools, FSHost } from '../interfaces'; import deploy, { deployToFunction } from './actions'; @@ -10,9 +10,12 @@ let fsHost: FSHost; const FIREBASE_PROJECT = 'ikachu-aa3ef'; const PROJECT = 'pirojok-project'; -const BUILD_TARGET: BuildTarget = { +const STATIC_BUILD_TARGET: BuildTarget = { name: `${PROJECT}:build:production` }; +const SERVER_BUILD_TARGET: BuildTarget = { + name: `${PROJECT}:server:production` +}; const initMocks = () => { fsHost = { @@ -53,7 +56,13 @@ const initMocks = () => { id: 1, logger: new logging.NullLogger() as any, workspaceRoot: 'cwd', - getTargetOptions: (_: Target) => Promise.resolve({}), + getTargetOptions: async (target: Target) => { + if (target.target === 'build') { + return { outputPath: 'dist/browser' }; + } else if (target.target === 'server') { + return { outputPath: 'dist/server' }; + } + }, reportProgress: (_: number, __?: number, ___?: string) => { }, reportStatus: (_: string) => { @@ -65,32 +74,18 @@ const initMocks = () => { } as any); }; - -const projectTargets: experimental.workspace.WorkspaceTool = { - build: { - options: { - outputPath: 'dist/browser' - } - }, - server: { - options: { - outputPath: 'dist/server' - } - } -}; - describe('Deploy Angular apps', () => { beforeEach(() => initMocks()); it('should call login', async () => { const spy = spyOn(firebaseMock, 'login'); - await deploy(firebaseMock, context, projectTargets, [BUILD_TARGET], FIREBASE_PROJECT, false, false); + await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, FIREBASE_PROJECT, false); expect(spy).toHaveBeenCalled(); }); it('should invoke the builder', async () => { const spy = spyOn(context, 'scheduleTarget').and.callThrough(); - await deploy(firebaseMock, context, projectTargets, [BUILD_TARGET], FIREBASE_PROJECT, false, false); + await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, FIREBASE_PROJECT, false); expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith({ target: 'build', @@ -105,14 +100,14 @@ describe('Deploy Angular apps', () => { options: {} }; const spy = spyOn(context, 'scheduleTarget').and.callThrough(); - await deploy(firebaseMock, context, projectTargets, [buildTarget], FIREBASE_PROJECT, false, false); + await deploy(firebaseMock, context, buildTarget, undefined, FIREBASE_PROJECT, false); expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith({ target: 'prerender', project: PROJECT }, {}); }); it('should invoke firebase.deploy', async () => { const spy = spyOn(firebaseMock, 'deploy').and.callThrough(); - await deploy(firebaseMock, context, projectTargets, [BUILD_TARGET], FIREBASE_PROJECT, false, false); + await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, FIREBASE_PROJECT, false); expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith({ cwd: 'cwd', @@ -123,7 +118,7 @@ describe('Deploy Angular apps', () => { describe('error handling', () => { it('throws if there is no firebase project', async () => { try { - await deploy(firebaseMock, context, projectTargets, [BUILD_TARGET], undefined, false, false); + await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, undefined, false); } catch (e) { console.log(e); expect(e.message).toMatch(/Cannot find firebase project/); @@ -133,7 +128,7 @@ describe('Deploy Angular apps', () => { it('throws if there is no target project', async () => { context.target = undefined; try { - await deploy(firebaseMock, context, projectTargets, [BUILD_TARGET], FIREBASE_PROJECT, false, false); + await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, FIREBASE_PROJECT, false); } catch (e) { expect(e.message).toMatch(/Cannot execute the build target/); } @@ -146,7 +141,7 @@ describe('universal deployment', () => { it('should create a firebase function', async () => { const spy = spyOn(fsHost, 'writeFileSync'); - await deployToFunction(firebaseMock, context, '/home/user', projectTargets, false, fsHost); + await deployToFunction(firebaseMock, context, '/home/user', STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, false, fsHost); expect(spy).toHaveBeenCalledTimes(2); @@ -159,7 +154,7 @@ describe('universal deployment', () => { it('should rename the index.html file in the nested dist', async () => { const spy = spyOn(fsHost, 'renameSync'); - await deployToFunction(firebaseMock, context, '/home/user', projectTargets, false, fsHost); + await deployToFunction(firebaseMock, context, '/home/user', STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, false, fsHost); expect(spy).toHaveBeenCalledTimes(1); @@ -173,7 +168,7 @@ describe('universal deployment', () => { it('should invoke firebase.deploy', async () => { const spy = spyOn(firebaseMock, 'deploy'); - await deployToFunction(firebaseMock, context, '/home/user', projectTargets, false, fsHost); + await deployToFunction(firebaseMock, context, '/home/user', STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, false, fsHost); expect(spy).toHaveBeenCalledTimes(1); }); diff --git a/src/schematics/deploy/actions.ts b/src/schematics/deploy/actions.ts index 0a7bab9bf..3d81e4bf2 100644 --- a/src/schematics/deploy/actions.ts +++ b/src/schematics/deploy/actions.ts @@ -5,8 +5,6 @@ import { copySync, removeSync } from 'fs-extra'; import { dirname, join } from 'path'; import { execSync } from 'child_process'; import { defaultFunction, defaultPackage, NODE_VERSION } from './functions-templates'; -import { experimental } from '@angular-devkit/core'; -import { SchematicsException } from '@angular-devkit/schematics'; import { satisfies } from 'semver'; import * as open from 'open'; @@ -117,7 +115,8 @@ export const deployToFunction = async ( firebaseTools: FirebaseTools, context: BuilderContext, workspaceRoot: string, - project: experimental.workspace.WorkspaceTool, + staticBuildTarget: BuildTarget, + serverBuildTarget: BuildTarget, preview: boolean, fsHost: FSHost = defaultFsHost ) => { @@ -127,30 +126,22 @@ export const deployToFunction = async ( ); } - if ( - !project || - !project.build || - !project.build.options || - !project.build.options.outputPath - ) { - throw new SchematicsException( - `Cannot read the output path (architect.build.options.outputPath) of the Angular project in angular.json` + const staticBuildOptions = await context.getTargetOptions(targetFromTargetString(staticBuildTarget.name)); + if (!staticBuildOptions.outputPath || typeof staticBuildOptions.outputPath !== 'string') { + throw new Error( + `Cannot read the output path option of the Angular project '${staticBuildTarget.name}' in angular.json` ); } - if ( - !project || - !project.server || - !project.server.options || - !project.server.options.outputPath - ) { - throw new SchematicsException( - `Cannot read the output path (architect.server.options.outputPath) of the Angular project in angular.json` + const serverBuildOptions = await context.getTargetOptions(targetFromTargetString(serverBuildTarget.name)); + if (!serverBuildOptions.outputPath || typeof serverBuildOptions.outputPath !== 'string') { + throw new Error( + `Cannot read the output path option of the Angular project '${serverBuildTarget.name}' in angular.json` ); } - const staticOut = project.build.options.outputPath; - const serverOut = project.server.options.outputPath; + const staticOut = staticBuildOptions.outputPath; + const serverOut = serverBuildOptions.outputPath; const newClientPath = join(dirname(staticOut), staticOut); const newServerPath = join(dirname(serverOut), serverOut); @@ -212,10 +203,9 @@ export const deployToFunction = async ( export default async function deploy( firebaseTools: FirebaseTools, context: BuilderContext, - projectTargets: experimental.workspace.WorkspaceTool, - buildTargets: BuildTarget[], + staticBuildTarget: BuildTarget, + serverBuildTarget: BuildTarget | undefined, firebaseProject: string, - ssr: boolean, preview: boolean ) { await firebaseTools.login(); @@ -226,10 +216,16 @@ export default async function deploy( context.logger.info(`📦 Building "${context.target.project}"`); - for (const target of buildTargets) { + const run = await context.scheduleTarget( + targetFromTargetString(staticBuildTarget.name), + staticBuildTarget.options + ); + await run.result; + + if (serverBuildTarget) { const run = await context.scheduleTarget( - targetFromTargetString(target.name), - target.options + targetFromTargetString(serverBuildTarget.name), + serverBuildTarget.options ); await run.result; } @@ -255,12 +251,13 @@ export default async function deploy( }) ); - if (ssr) { + if (serverBuildTarget) { await deployToFunction( firebaseTools, context, context.workspaceRoot, - projectTargets, + staticBuildTarget, + serverBuildTarget, preview ); } else { diff --git a/src/schematics/deploy/builder.ts b/src/schematics/deploy/builder.ts index b47b9e130..0242b0e9d 100644 --- a/src/schematics/deploy/builder.ts +++ b/src/schematics/deploy/builder.ts @@ -1,32 +1,18 @@ import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect'; -import { NodeJsSyncHost } from '@angular-devkit/core/node'; import deploy from './actions'; -import { experimental, json, normalize } from '@angular-devkit/core'; import { BuildTarget, DeployBuilderSchema } from '../interfaces'; import { getFirebaseProjectName } from '../utils'; -type DeployBuilderOptions = DeployBuilderSchema & json.JsonObject; +type DeployBuilderOptions = DeployBuilderSchema & Record; // Call the createBuilder() function to create a builder. This mirrors // createJobHandler() but add typings specific to Architect Builders. -export default createBuilder( +export default createBuilder( async (options: DeployBuilderOptions, context: BuilderContext): Promise => { - // The project root is added to a BuilderContext. - const root = normalize(context.workspaceRoot); - const workspace = new experimental.workspace.Workspace( - root, - new NodeJsSyncHost() - ); - await workspace - .loadWorkspaceFromHost(normalize('angular.json')) - .toPromise(); - if (!context.target) { throw new Error('Cannot deploy the application without a target'); } - const projectTargets = workspace.getProjectTargets(context.target.project); - const firebaseProject = getFirebaseProjectName( context.workspaceRoot, context.target.project @@ -36,28 +22,25 @@ export default createBuilder( throw new Error('Cannot find firebase project for your app in .firebaserc'); } - const buildTarget = options.buildTarget || `${context.target.project}:build:production`; + const staticBuildTarget = { name: options.buildTarget || `${context.target.project}:build:production` }; - const targets: BuildTarget[] = [{ - name: buildTarget - }]; + let serverBuildTarget: BuildTarget | undefined; if (options.ssr) { - targets.push({ + serverBuildTarget = { name: options.universalBuildTarget || `${context.target.project}:server:production`, options: { bundleDependencies: 'all' } - }); + }; } try { await deploy( require('firebase-tools'), context, - projectTargets, - targets, + staticBuildTarget, + serverBuildTarget, firebaseProject, - !!options.ssr, !!options.preview ); } catch (e) {