Skip to content

fix(deploy): remove direct workspace access #2643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 21 additions & 26 deletions src/schematics/deploy/actions.jasmine.ts
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
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
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