Skip to content

Commit 7e1918a

Browse files
clydinclydin
authored
fix(deploy): remove direct workspace access (#2643)
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.
1 parent e5743da commit 7e1918a

File tree

3 files changed

+55
-80
lines changed

3 files changed

+55
-80
lines changed

‎src/schematics/deploy/actions.jasmine.ts‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { experimental, JsonObject, logging } from '@angular-devkit/core';
1+
import { JsonObject, logging } from '@angular-devkit/core';
22
import { BuilderContext, BuilderRun, ScheduleOptions, Target } from '@angular-devkit/architect';
33
import { BuildTarget, FirebaseDeployConfig, FirebaseTools, FSHost } from '../interfaces';
44
import deploy, { deployToFunction } from './actions';
@@ -10,9 +10,12 @@ let fsHost: FSHost;
1010

1111
const FIREBASE_PROJECT = 'ikachu-aa3ef';
1212
const PROJECT = 'pirojok-project';
13-
const BUILD_TARGET: BuildTarget = {
13+
const STATIC_BUILD_TARGET: BuildTarget = {
1414
name: `${PROJECT}:build:production`
1515
};
16+
const SERVER_BUILD_TARGET: BuildTarget = {
17+
name: `${PROJECT}:server:production`
18+
};
1619

1720
const initMocks = () => {
1821
fsHost = {
@@ -53,7 +56,13 @@ const initMocks = () => {
5356
id: 1,
5457
logger: new logging.NullLogger() as any,
5558
workspaceRoot: 'cwd',
56-
getTargetOptions: (_: Target) => Promise.resolve({}),
59+
getTargetOptions: async (target: Target) => {
60+
if (target.target === 'build') {
61+
return { outputPath: 'dist/browser' };
62+
} else if (target.target === 'server') {
63+
return { outputPath: 'dist/server' };
64+
}
65+
},
5766
reportProgress: (_: number, __?: number, ___?: string) => {
5867
},
5968
reportStatus: (_: string) => {
@@ -65,32 +74,18 @@ const initMocks = () => {
6574
} as any);
6675
};
6776

68-
69-
const projectTargets: experimental.workspace.WorkspaceTool = {
70-
build: {
71-
options: {
72-
outputPath: 'dist/browser'
73-
}
74-
},
75-
server: {
76-
options: {
77-
outputPath: 'dist/server'
78-
}
79-
}
80-
};
81-
8277
describe('Deploy Angular apps', () => {
8378
beforeEach(() => initMocks());
8479

8580
it('should call login', async () => {
8681
const spy = spyOn(firebaseMock, 'login');
87-
await deploy(firebaseMock, context, projectTargets, [BUILD_TARGET], FIREBASE_PROJECT, false, false);
82+
await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, FIREBASE_PROJECT, false);
8883
expect(spy).toHaveBeenCalled();
8984
});
9085

9186
it('should invoke the builder', async () => {
9287
const spy = spyOn(context, 'scheduleTarget').and.callThrough();
93-
await deploy(firebaseMock, context, projectTargets, [BUILD_TARGET], FIREBASE_PROJECT, false, false);
88+
await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, FIREBASE_PROJECT, false);
9489
expect(spy).toHaveBeenCalled();
9590
expect(spy).toHaveBeenCalledWith({
9691
target: 'build',
@@ -105,14 +100,14 @@ describe('Deploy Angular apps', () => {
105100
options: {}
106101
};
107102
const spy = spyOn(context, 'scheduleTarget').and.callThrough();
108-
await deploy(firebaseMock, context, projectTargets, [buildTarget], FIREBASE_PROJECT, false, false);
103+
await deploy(firebaseMock, context, buildTarget, undefined, FIREBASE_PROJECT, false);
109104
expect(spy).toHaveBeenCalled();
110105
expect(spy).toHaveBeenCalledWith({ target: 'prerender', project: PROJECT }, {});
111106
});
112107

113108
it('should invoke firebase.deploy', async () => {
114109
const spy = spyOn(firebaseMock, 'deploy').and.callThrough();
115-
await deploy(firebaseMock, context, projectTargets, [BUILD_TARGET], FIREBASE_PROJECT, false, false);
110+
await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, FIREBASE_PROJECT, false);
116111
expect(spy).toHaveBeenCalled();
117112
expect(spy).toHaveBeenCalledWith({
118113
cwd: 'cwd',
@@ -123,7 +118,7 @@ describe('Deploy Angular apps', () => {
123118
describe('error handling', () => {
124119
it('throws if there is no firebase project', async () => {
125120
try {
126-
await deploy(firebaseMock, context, projectTargets, [BUILD_TARGET], undefined, false, false);
121+
await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, undefined, false);
127122
} catch (e) {
128123
console.log(e);
129124
expect(e.message).toMatch(/Cannot find firebase project/);
@@ -133,7 +128,7 @@ describe('Deploy Angular apps', () => {
133128
it('throws if there is no target project', async () => {
134129
context.target = undefined;
135130
try {
136-
await deploy(firebaseMock, context, projectTargets, [BUILD_TARGET], FIREBASE_PROJECT, false, false);
131+
await deploy(firebaseMock, context, STATIC_BUILD_TARGET, undefined, FIREBASE_PROJECT, false);
137132
} catch (e) {
138133
expect(e.message).toMatch(/Cannot execute the build target/);
139134
}
@@ -146,7 +141,7 @@ describe('universal deployment', () => {
146141

147142
it('should create a firebase function', async () => {
148143
const spy = spyOn(fsHost, 'writeFileSync');
149-
await deployToFunction(firebaseMock, context, '/home/user', projectTargets, false, fsHost);
144+
await deployToFunction(firebaseMock, context, '/home/user', STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, false, fsHost);
150145

151146
expect(spy).toHaveBeenCalledTimes(2);
152147

@@ -159,7 +154,7 @@ describe('universal deployment', () => {
159154

160155
it('should rename the index.html file in the nested dist', async () => {
161156
const spy = spyOn(fsHost, 'renameSync');
162-
await deployToFunction(firebaseMock, context, '/home/user', projectTargets, false, fsHost);
157+
await deployToFunction(firebaseMock, context, '/home/user', STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, false, fsHost);
163158

164159
expect(spy).toHaveBeenCalledTimes(1);
165160

@@ -173,7 +168,7 @@ describe('universal deployment', () => {
173168

174169
it('should invoke firebase.deploy', async () => {
175170
const spy = spyOn(firebaseMock, 'deploy');
176-
await deployToFunction(firebaseMock, context, '/home/user', projectTargets, false, fsHost);
171+
await deployToFunction(firebaseMock, context, '/home/user', STATIC_BUILD_TARGET, SERVER_BUILD_TARGET, false, fsHost);
177172

178173
expect(spy).toHaveBeenCalledTimes(1);
179174
});

‎src/schematics/deploy/actions.ts‎

Lines changed: 26 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import { copySync, removeSync } from 'fs-extra';
55
import { dirname, join } from 'path';
66
import { execSync } from 'child_process';
77
import { defaultFunction, defaultPackage, NODE_VERSION } from './functions-templates';
8-
import { experimental } from '@angular-devkit/core';
9-
import { SchematicsException } from '@angular-devkit/schematics';
108
import { satisfies } from 'semver';
119
import * as open from 'open';
1210

@@ -117,7 +115,8 @@ export const deployToFunction = async (
117115
firebaseTools: FirebaseTools,
118116
context: BuilderContext,
119117
workspaceRoot: string,
120-
project: experimental.workspace.WorkspaceTool,
118+
staticBuildTarget: BuildTarget,
119+
serverBuildTarget: BuildTarget,
121120
preview: boolean,
122121
fsHost: FSHost = defaultFsHost
123122
) => {
@@ -127,30 +126,22 @@ export const deployToFunction = async (
127126
);
128127
}
129128

130-
if (
131-
!project ||
132-
!project.build ||
133-
!project.build.options ||
134-
!project.build.options.outputPath
135-
) {
136-
throw new SchematicsException(
137-
`Cannot read the output path (architect.build.options.outputPath) of the Angular project in angular.json`
129+
const staticBuildOptions = await context.getTargetOptions(targetFromTargetString(staticBuildTarget.name));
130+
if (!staticBuildOptions.outputPath || typeof staticBuildOptions.outputPath !== 'string') {
131+
throw new Error(
132+
`Cannot read the output path option of the Angular project '${staticBuildTarget.name}' in angular.json`
138133
);
139134
}
140135

141-
if (
142-
!project ||
143-
!project.server ||
144-
!project.server.options ||
145-
!project.server.options.outputPath
146-
) {
147-
throw new SchematicsException(
148-
`Cannot read the output path (architect.server.options.outputPath) of the Angular project in angular.json`
136+
const serverBuildOptions = await context.getTargetOptions(targetFromTargetString(serverBuildTarget.name));
137+
if (!serverBuildOptions.outputPath || typeof serverBuildOptions.outputPath !== 'string') {
138+
throw new Error(
139+
`Cannot read the output path option of the Angular project '${serverBuildTarget.name}' in angular.json`
149140
);
150141
}
151142

152-
const staticOut = project.build.options.outputPath;
153-
const serverOut = project.server.options.outputPath;
143+
const staticOut = staticBuildOptions.outputPath;
144+
const serverOut = serverBuildOptions.outputPath;
154145
const newClientPath = join(dirname(staticOut), staticOut);
155146
const newServerPath = join(dirname(serverOut), serverOut);
156147

@@ -212,10 +203,9 @@ export const deployToFunction = async (
212203
export default async function deploy(
213204
firebaseTools: FirebaseTools,
214205
context: BuilderContext,
215-
projectTargets: experimental.workspace.WorkspaceTool,
216-
buildTargets: BuildTarget[],
206+
staticBuildTarget: BuildTarget,
207+
serverBuildTarget: BuildTarget | undefined,
217208
firebaseProject: string,
218-
ssr: boolean,
219209
preview: boolean
220210
) {
221211
await firebaseTools.login();
@@ -226,10 +216,16 @@ export default async function deploy(
226216

227217
context.logger.info(`📦 Building "${context.target.project}"`);
228218

229-
for (const target of buildTargets) {
219+
const run = await context.scheduleTarget(
220+
targetFromTargetString(staticBuildTarget.name),
221+
staticBuildTarget.options
222+
);
223+
await run.result;
224+
225+
if (serverBuildTarget) {
230226
const run = await context.scheduleTarget(
231-
targetFromTargetString(target.name),
232-
target.options
227+
targetFromTargetString(serverBuildTarget.name),
228+
serverBuildTarget.options
233229
);
234230
await run.result;
235231
}
@@ -255,12 +251,13 @@ export default async function deploy(
255251
})
256252
);
257253

258-
if (ssr) {
254+
if (serverBuildTarget) {
259255
await deployToFunction(
260256
firebaseTools,
261257
context,
262258
context.workspaceRoot,
263-
projectTargets,
259+
staticBuildTarget,
260+
serverBuildTarget,
264261
preview
265262
);
266263
} else {

‎src/schematics/deploy/builder.ts‎

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,18 @@
11
import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
2-
import { NodeJsSyncHost } from '@angular-devkit/core/node';
32
import deploy from './actions';
4-
import { experimental, json, normalize } from '@angular-devkit/core';
53
import { BuildTarget, DeployBuilderSchema } from '../interfaces';
64
import { getFirebaseProjectName } from '../utils';
75

8-
type DeployBuilderOptions = DeployBuilderSchema & json.JsonObject;
6+
type DeployBuilderOptions = DeployBuilderSchema & Record<string, string>;
97

108
// Call the createBuilder() function to create a builder. This mirrors
119
// createJobHandler() but add typings specific to Architect Builders.
12-
export default createBuilder<any>(
10+
export default createBuilder(
1311
async (options: DeployBuilderOptions, context: BuilderContext): Promise<BuilderOutput> => {
14-
// The project root is added to a BuilderContext.
15-
const root = normalize(context.workspaceRoot);
16-
const workspace = new experimental.workspace.Workspace(
17-
root,
18-
new NodeJsSyncHost()
19-
);
20-
await workspace
21-
.loadWorkspaceFromHost(normalize('angular.json'))
22-
.toPromise();
23-
2412
if (!context.target) {
2513
throw new Error('Cannot deploy the application without a target');
2614
}
2715

28-
const projectTargets = workspace.getProjectTargets(context.target.project);
29-
3016
const firebaseProject = getFirebaseProjectName(
3117
context.workspaceRoot,
3218
context.target.project
@@ -36,28 +22,25 @@ export default createBuilder<any>(
3622
throw new Error('Cannot find firebase project for your app in .firebaserc');
3723
}
3824

39-
const buildTarget = options.buildTarget || `${context.target.project}:build:production`;
25+
const staticBuildTarget = { name: options.buildTarget || `${context.target.project}:build:production` };
4026

41-
const targets: BuildTarget[] = [{
42-
name: buildTarget
43-
}];
27+
let serverBuildTarget: BuildTarget | undefined;
4428
if (options.ssr) {
45-
targets.push({
29+
serverBuildTarget = {
4630
name: options.universalBuildTarget || `${context.target.project}:server:production`,
4731
options: {
4832
bundleDependencies: 'all'
4933
}
50-
});
34+
};
5135
}
5236

5337
try {
5438
await deploy(
5539
require('firebase-tools'),
5640
context,
57-
projectTargets,
58-
targets,
41+
staticBuildTarget,
42+
serverBuildTarget,
5943
firebaseProject,
60-
!!options.ssr,
6144
!!options.preview
6245
);
6346
} catch (e) {

0 commit comments

Comments
 (0)