Skip to content

Commit

Permalink
feat(@angular-devkit/build-angular): emit external sourcemaps for com…
Browse files Browse the repository at this point in the history
…ponent styles

This commits, changes the behaviour in the esbuild based builders by emitting component sourcemaps in external files instead of inlining them.

Closes angular#24049 and closes angular#26676
  • Loading branch information
alan-agius4 committed Dec 20, 2023
1 parent a708dcc commit e406cb2
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -136,5 +136,41 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {

harness.expectFile('dist/browser/main.js.map').content.toContain('"x_google_ignoreList"');
});

it('should generate component sourcemaps when sourcemaps when true', async () => {
await harness.writeFile('src/app/app.component.css', `* { color: red}`);

harness.useTarget('build', {
...BASE_OPTIONS,
sourceMap: true,
});

const { result } = await harness.executeOnce();

expect(result?.success).toBeTrue();

harness
.expectFile('dist/browser/main.js')
.content.toContain('sourceMappingURL=app.component.css.map');
harness.expectFile('dist/browser/app.component.css.map').toExist();
});

it('should not generate component sourcemaps when sourcemaps when false', async () => {
await harness.writeFile('src/app/app.component.css', `* { color: red}`);

harness.useTarget('build', {
...BASE_OPTIONS,
sourceMap: false,
});

const { result } = await harness.executeOnce();

expect(result?.success).toBeTrue();

harness
.expectFile('dist/browser/main.js')
.content.not.toContain('sourceMappingURL=app.component.css.map');
harness.expectFile('dist/browser/app.component.css.map').toNotExist();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,15 @@ export function createCompilerPlugin(
);
}

const { contents, resourceFiles, referencedFiles, errors, warnings } = stylesheetResult;
const { contents, outputFiles, metafile, referencedFiles, errors, warnings } =
stylesheetResult;
if (errors) {
(result.errors ??= []).push(...errors);
}
(result.warnings ??= []).push(...warnings);
additionalResults.set(stylesheetFile ?? containingFile, {
outputFiles: resourceFiles,
metafile: stylesheetResult.metafile,
outputFiles,
metafile,
});

if (referencedFiles) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class ComponentStylesheetBundler {
});
});

return extractResult(await bundlerContext.bundle(), bundlerContext.watchFiles);
return this.extractResult(await bundlerContext.bundle(), bundlerContext.watchFiles);
}

async bundleInline(data: string, filename: string, language: string) {
Expand Down Expand Up @@ -103,7 +103,7 @@ export class ComponentStylesheetBundler {
});

// Extract the result of the bundling from the output files
return extractResult(await bundlerContext.bundle(), bundlerContext.watchFiles);
return this.extractResult(await bundlerContext.bundle(), bundlerContext.watchFiles);
}

invalidate(files: Iterable<string>) {
Expand All @@ -128,52 +128,51 @@ export class ComponentStylesheetBundler {

await Promise.allSettled(contexts.map((context) => context.dispose()));
}
}

function extractResult(result: BundleContextResult, referencedFiles?: Set<string>) {
let contents = '';
let map;
let outputPath;
const resourceFiles: OutputFile[] = [];
if (!result.errors) {
for (const outputFile of result.outputFiles) {
const filename = path.basename(outputFile.path);
if (outputFile.type === BuildOutputFileType.Media) {
// The output files could also contain resources (images/fonts/etc.) that were referenced
resourceFiles.push(outputFile);
} else if (filename.endsWith('.css')) {
outputPath = outputFile.path;
contents = outputFile.text;
} else if (filename.endsWith('.css.map')) {
map = outputFile.text;
} else {
throw new Error(
`Unexpected non CSS/Media file "${filename}" outputted during component stylesheet processing.`,
);
private extractResult(result: BundleContextResult, referencedFiles?: Set<string>) {
let contents = '';
const outputFiles: OutputFile[] = [];
if (!result.errors) {
for (const outputFile of result.outputFiles) {
const filename = path.basename(outputFile.path);

// Needed for Bazel as otherwise the files will not be written in the correct place.
outputFile.path = path.join(this.options.workspaceRoot, outputFile.path);

if (outputFile.type === BuildOutputFileType.Media) {
// The output files could also contain resources (images/fonts/etc.) that were referenced
outputFiles.push(outputFile);
} else if (filename.endsWith('.css')) {
contents = outputFile.text;
} else if (filename.endsWith('.css.map')) {
outputFiles.push(outputFile);
} else {
throw new Error(
`Unexpected non CSS/Media file "${filename}" outputted during component stylesheet processing.`,
);
}
}
}
}

let metafile;
if (!result.errors) {
metafile = result.metafile;
// Remove entryPoint fields from outputs to prevent the internal component styles from being
// treated as initial files. Also mark the entry as a component resource for stat reporting.
Object.values(metafile.outputs).forEach((output) => {
delete output.entryPoint;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(output as any)['ng-component'] = true;
});
}
let metafile;
if (!result.errors) {
metafile = result.metafile;
// Remove entryPoint fields from outputs to prevent the internal component styles from being
// treated as initial files. Also mark the entry as a component resource for stat reporting.
Object.values(metafile.outputs).forEach((output) => {
delete output.entryPoint;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(output as any)['ng-component'] = true;
});
}

return {
errors: result.errors,
warnings: result.warnings,
contents,
map,
path: outputPath,
resourceFiles,
metafile,
referencedFiles,
};
return {
errors: result.errors,
warnings: result.warnings,
contents,
outputFiles,
metafile,
referencedFiles,
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,10 @@ export function setupJitPluginCallbacks(
);
}

const { contents, resourceFiles, errors, warnings, metafile, referencedFiles } =
const { contents, outputFiles, errors, warnings, metafile, referencedFiles } =
stylesheetResult;

additionalResultFiles.set(entry.path, { outputFiles: resourceFiles, metafile });
additionalResultFiles.set(entry.path, { outputFiles, metafile });

return {
errors,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export function createCompilerPluginOptions(
// Hidden component stylesheet sourcemaps are inaccessible which is effectively
// the same as being disabled. Disabling has the advantage of avoiding the overhead
// of sourcemap processing.
!!sourcemapOptions.styles && (sourcemapOptions.hidden ? false : 'inline'),
sourcemapOptions.styles && !sourcemapOptions.hidden ? 'linked' : false,
outputNames,
includePaths: stylePreprocessorOptions?.includePaths,
externalDependencies,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export interface BundleStylesheetOptions {
optimization: boolean;
inlineFonts: boolean;
preserveSymlinks?: boolean;
sourcemap: boolean | 'external' | 'inline';
sourcemap: boolean | 'external' | 'inline' | 'linked';
outputNames: { bundles: string; media: string };
includePaths?: string[];
externalDependencies?: string[];
Expand Down

0 comments on commit e406cb2

Please sign in to comment.