Skip to content

Commit

Permalink
fix(deploy): remove direct workspace access (#2643)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@clydin
clydin authored Nov 11, 2020
1 parent e5743da commit 7e1918a
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 80 deletions.
47 changes: 21 additions & 26 deletions src/schematics/deploy/actions.jasmine.ts
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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 = {
Expand Down Expand Up @@ -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) => {
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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/);
Expand All @@ -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/);
}
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);
});
Expand Down
55 changes: 26 additions & 29 deletions src/schematics/deploy/actions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
) => {
Expand All @@ -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);

Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
Expand All @@ -255,12 +251,13 @@ export default async function deploy(
})
);

if (ssr) {
if (serverBuildTarget) {
await deployToFunction(
firebaseTools,
context,
context.workspaceRoot,
projectTargets,
staticBuildTarget,
serverBuildTarget,
preview
);
} else {
Expand Down
33 changes: 8 additions & 25 deletions src/schematics/deploy/builder.ts
View file
Original file line number Diff line number Diff line change
@@ -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<string, string>;

// Call the createBuilder() function to create a builder. This mirrors
// createJobHandler() but add typings specific to Architect Builders.
export default createBuilder<any>(
export default createBuilder(
async (options: DeployBuilderOptions, context: BuilderContext): Promise<BuilderOutput> => {
// 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
Expand All @@ -36,28 +22,25 @@ export default createBuilder<any>(
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) {
Expand Down

0 comments on commit 7e1918a

Please sign in to comment.