Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): ensure correct web worker URL res…
Browse files Browse the repository at this point in the history 
…olution in vite dev server

When using the application builder with the development server, Web Worker URLs previously may
have been incorrectly resolved. This caused Vite to consider the Web Worker URLs as outside
the project root and generate a special file system URL. While this worked on Mac/Linux, it
would fail on Windows. Since Vite does not appear to support resolve plugins for Web Workers,
the virtual project root for the in-memory build has now been adjusted to allow the referencing
file to have a path that resolves the Web Worker URL to a project relative location.
  • Loading branch information
@clydin @alan-agius4
clydin authored and alan-agius4 committed Oct 30, 2023
1 parent 44ea76a commit 09682e5
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 16 deletions.
17 changes: 17 additions & 0 deletions ..._devkit/build_angular/src/builders/application/tests/behavior/rebuild-web-workers_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ import { APPLICATION_BUILDER_INFO, BASE_OPTIONS, describeBuilder } from '../setu
*/
export const BUILD_TIMEOUT = 30_000;

/**
* A regular expression used to check if a built worker is correctly referenced in application code.
*/
const REFERENCED_WORKER_REGEXP =
/new Worker\(new URL\("worker-[A-Z0-9]{8}\.js", import\.meta\.url\)/;

describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
describe('Behavior: "Rebuilds when Web Worker files change"', () => {
it('Recovers from error when directly referenced worker file is changed', async () => {
Expand Down Expand Up @@ -59,11 +65,17 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
case 0:
expect(result?.success).toBeTrue();

// Ensure built worker is referenced in the application code
harness
.expectFile('dist/browser/main.js')
.content.toMatch(REFERENCED_WORKER_REGEXP);

// Update the worker file to be invalid syntax
await harness.writeFile('src/app/worker.ts', `asd;fj$3~kls;kd^(*fjlk;sdj---flk`);

break;
case 1:
expect(result?.success).toBeFalse();
expect(logs).toContain(
jasmine.objectContaining<logging.LogEntry>({
message: jasmine.stringMatching(errorText),
Expand Down Expand Up @@ -108,6 +120,11 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
}),
);

// Ensure built worker is referenced in the application code
harness
.expectFile('dist/browser/main.js')
.content.toMatch(REFERENCED_WORKER_REGEXP);

// Test complete - abort watch mode
builderAbort?.abort();
break;
Expand Down
42 changes: 27 additions & 15 deletions packages/angular_devkit/build_angular/src/builders/dev-server/vite-server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type { json, logging } from '@angular-devkit/core';
import type { Plugin } from 'esbuild';
import { lookup as lookupMimeType } from 'mrmime';
import assert from 'node:assert';
import { randomUUID } from 'node:crypto';
import { readFile } from 'node:fs/promises';
import { ServerResponse } from 'node:http';
import type { AddressInfo } from 'node:net';
Expand Down Expand Up @@ -182,7 +183,7 @@ export async function* serveWithVite(
}

if (server) {
handleUpdate(generatedFiles, server, serverOptions, context.logger);
handleUpdate(normalizePath, generatedFiles, server, serverOptions, context.logger);
} else {
const projectName = context.target?.project;
if (!projectName) {
Expand Down Expand Up @@ -230,6 +231,7 @@ export async function* serveWithVite(
}

function handleUpdate(
normalizePath: (id: string) => string,
generatedFiles: Map<string, OutputFileRecord>,
server: ViteDevServer,
serverOptions: NormalizedDevServerOptions,
Expand All @@ -241,7 +243,9 @@ function handleUpdate(
for (const [file, record] of generatedFiles) {
if (record.updated) {
updatedFiles.push(file);
const updatedModules = server.moduleGraph.getModulesByFile(file);
const updatedModules = server.moduleGraph.getModulesByFile(
normalizePath(path.join(server.config.root, file)),
);
updatedModules?.forEach((m) => server?.moduleGraph.invalidateModule(m));
}
}
Expand All @@ -255,9 +259,7 @@ function handleUpdate(
const timestamp = Date.now();
server.ws.send({
type: 'update',
updates: updatedFiles.map((f) => {
const filePath = f.slice(1); // Remove leading slash.

updates: updatedFiles.map((filePath) => {
return {
type: 'css-update',
timestamp,
Expand Down Expand Up @@ -298,7 +300,7 @@ function analyzeResultFiles(
// This mimics the Webpack dev-server behavior.
filePath = '/index.html';
} else {
filePath = '/' + normalizePath(file.path);
filePath = normalizePath(file.path);
}
seen.add(filePath);

Expand Down Expand Up @@ -365,11 +367,16 @@ export async function setupServer(
// dynamically import Vite for ESM compatibility
const { normalizePath } = await import('vite');

// Path will not exist on disk and only used to provide separate path for Vite requests
const virtualProjectRoot = normalizePath(
path.join(serverOptions.workspaceRoot, `.angular/vite-root/${randomUUID()}/`),
);

const configuration: InlineConfig = {
configFile: false,
envFile: false,
cacheDir: path.join(serverOptions.cacheOptions.path, 'vite'),
root: serverOptions.workspaceRoot,
root: virtualProjectRoot,
publicDir: false,
esbuild: false,
mode: 'development',
Expand Down Expand Up @@ -399,7 +406,7 @@ export async function setupServer(
},
ssr: {
// Exclude any provided dependencies (currently build defined externals)
external: externalMetadata.implicit,
external: externalMetadata.explicit,
},
plugins: [
createAngularLocaleDataPlugin(),
Expand All @@ -415,27 +422,32 @@ export async function setupServer(
return source;
}

if (importer && source.startsWith('.')) {
if (importer && source[0] === '.' && importer.startsWith(virtualProjectRoot)) {
// Remove query if present
const [importerFile] = importer.split('?', 1);

source = normalizePath(path.join(path.dirname(importerFile), source));
source = normalizePath(
path.join(path.dirname(path.relative(virtualProjectRoot, importerFile)), source),
);
}
if (source[0] === '/') {
source = source.slice(1);
}

const [file] = source.split('?', 1);
if (outputFiles.has(file)) {
return source;
return path.join(virtualProjectRoot, source);
}
},
load(id) {
const [file] = id.split('?', 1);
const codeContents = outputFiles.get(file)?.contents;
const relativeFile = normalizePath(path.relative(virtualProjectRoot, file));
const codeContents = outputFiles.get(relativeFile)?.contents;
if (codeContents === undefined) {
return;
}

const code = Buffer.from(codeContents).toString('utf-8');
const mapContents = outputFiles.get(file + '.map')?.contents;
const mapContents = outputFiles.get(relativeFile + '.map')?.contents;

return {
// Remove source map URL comments from the code if a sourcemap is present.
Expand Down Expand Up @@ -532,7 +544,7 @@ export async function setupServer(
return;
}

const rawHtml = outputFiles.get('/index.server.html')?.contents;
const rawHtml = outputFiles.get('index.server.html')?.contents;
if (!rawHtml) {
next();

Expand Down
6 changes: 5 additions & 1 deletion packages/angular_devkit/build_angular/src/tools/esbuild/angular/compiler-plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,12 @@ export function createCompilerPlugin(
file.path.endsWith('.js'),
);
assert(workerCodeFile, 'Web Worker bundled code file should always be present.');
const workerCodePath = path.relative(
build.initialOptions.outdir ?? '',
workerCodeFile.path,
);

return path.relative(build.initialOptions.outdir ?? '', workerCodeFile.path);
return workerCodePath.replaceAll('\\', '/');
},
};

Expand Down

0 comments on commit 09682e5

Please sign in to comment.