Skip to content

Commit

Permalink
feat(@schematics/angular): replace assets with public directory
Browse files Browse the repository at this point in the history
The `assets` directory is confusing for the users and commonly users place "assets" which are not meant to be copied but instead processed by the build system. This causes some files both bundled and copied.

With this change we rename the `assets` directory to `public` and also move the `favicon.ico` inside this newly created directory.
  • Loading branch information
alan-agius4 committed Mar 25, 2024
1 parent 9d3aa46 commit 1399e7f
Show file tree
Hide file tree
Showing 32 changed files with 101 additions and 117 deletions.
2 changes: 1 addition & 1 deletion goldens/public-api/angular_devkit/build_angular/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export interface AssetPatternObject {
glob: string;
ignore?: string[];
input: string;
output: string;
output?: string;
}

// @public
Expand Down
43 changes: 23 additions & 20 deletions packages/angular/pwa/pwa/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
externalSchematic,
mergeWith,
move,
template,
url,
} from '@angular-devkit/schematics';
import { readWorkspace, writeWorkspace } from '@schematics/angular/utility';
Expand Down Expand Up @@ -104,23 +103,6 @@ export default function (options: PwaOptions): Rule {
}
}

// Add manifest to asset configuration
const assetEntry = posix.join(
project.sourceRoot ?? posix.join(project.root, 'src'),
'manifest.webmanifest',
);
for (const target of [...buildTargets, ...testTargets]) {
if (target.options) {
if (Array.isArray(target.options.assets)) {
target.options.assets.push(assetEntry);
} else {
target.options.assets = [assetEntry];
}
} else {
target.options = { assets: [assetEntry] };
}
}

// Find all index.html files in build targets
const indexFiles = new Set<string>();
for (const target of buildTargets) {
Expand All @@ -146,11 +128,32 @@ export default function (options: PwaOptions): Rule {
const { title, ...swOptions } = options;

await writeWorkspace(host, workspace);
let assetsDir = posix.join(sourcePath, 'assets');

if (host.exists(assetsDir)) {
// Add manifest to asset configuration
const assetEntry = posix.join(
project.sourceRoot ?? posix.join(project.root, 'src'),
'manifest.webmanifest',
);
for (const target of [...buildTargets, ...testTargets]) {
if (target.options) {
if (Array.isArray(target.options.assets)) {
target.options.assets.push(assetEntry);
} else {
target.options.assets = [assetEntry];
}
} else {
target.options = { assets: [assetEntry] };
}
}
} else {
assetsDir = posix.join(project.root, 'public');
}

return chain([
externalSchematic('@schematics/angular', 'service-worker', swOptions),
mergeWith(apply(url('./files/root'), [template({ ...options }), move(sourcePath)])),
mergeWith(apply(url('./files/assets'), [move(posix.join(sourcePath, 'assets'))])),
mergeWith(apply(url('./files/assets'), [move(assetsDir)])),
...[...indexFiles].map((path) => updateIndexFile(path)),
]);
};
Expand Down
17 changes: 3 additions & 14 deletions packages/angular/pwa/pwa/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ describe('PWA Schematic', () => {

it('should create a manifest file', async () => {
const tree = await schematicRunner.runSchematic('ng-add', defaultOptions, appTree);
expect(tree.exists('/projects/bar/src/manifest.webmanifest')).toBeTrue();
expect(tree.exists('/projects/bar/public/manifest.webmanifest')).toBeTrue();
});

it('should set the name & short_name in the manifest file', async () => {
const tree = await schematicRunner.runSchematic('ng-add', defaultOptions, appTree);

const manifestText = tree.readContent('/projects/bar/src/manifest.webmanifest');
const manifestText = tree.readContent('/projects/bar/public/manifest.webmanifest');
const manifest = JSON.parse(manifestText);

expect(manifest.name).toEqual(defaultOptions.title);
Expand All @@ -91,7 +91,7 @@ describe('PWA Schematic', () => {
const options = { ...defaultOptions, title: undefined };
const tree = await schematicRunner.runSchematic('ng-add', options, appTree);

const manifestText = tree.readContent('/projects/bar/src/manifest.webmanifest');
const manifestText = tree.readContent('/projects/bar/public/manifest.webmanifest');
const manifest = JSON.parse(manifestText);

expect(manifest.name).toEqual(defaultOptions.project);
Expand Down Expand Up @@ -125,17 +125,6 @@ describe('PWA Schematic', () => {
expect(content).toMatch(/<noscript>NO JAVASCRIPT<\/noscript>/);
});

it('should update the build and test assets configuration', async () => {
const tree = await schematicRunner.runSchematic('ng-add', defaultOptions, appTree);
const configText = tree.readContent('/angular.json');
const config = JSON.parse(configText);
const targets = config.projects.bar.architect;

['build', 'test'].forEach((target) => {
expect(targets[target].options.assets).toContain('projects/bar/src/manifest.webmanifest');
});
});

describe('Legacy browser builder', () => {
function convertBuilderToLegacyBrowser(): void {
const config = JSON.parse(appTree.readContent('/angular.json'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -560,11 +560,12 @@
},
"output": {
"type": "string",
"default": "",
"description": "Absolute path within the output."
}
},
"additionalProperties": false,
"required": ["glob", "input", "output"]
"required": ["glob", "input"]
},
{
"type": "string"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,11 +466,12 @@
},
"output": {
"type": "string",
"default": "",
"description": "Absolute path within the output."
}
},
"additionalProperties": false,
"required": ["glob", "input", "output"]
"required": ["glob", "input"]
},
{
"type": "string"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,11 +454,12 @@
},
"output": {
"type": "string",
"default": "",
"description": "Absolute path within the output."
}
},
"additionalProperties": false,
"required": ["glob", "input", "output"]
"required": ["glob", "input"]
},
{
"type": "string"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@
},
"output": {
"type": "string",
"default": "",
"description": "Absolute path within the output."
},
"ignore": {
Expand All @@ -301,7 +302,7 @@
}
},
"additionalProperties": false,
"required": ["glob", "input", "output"]
"required": ["glob", "input"]
},
{
"type": "string"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,12 @@
},
"output": {
"type": "string",
"default": "",
"description": "Absolute path within the output."
}
},
"additionalProperties": false,
"required": ["glob", "input", "output"]
"required": ["glob", "input"]
},
{
"type": "string"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@
},
"output": {
"type": "string",
"default": "",
"description": "Absolute path within the output."
},
"ignore": {
Expand All @@ -280,7 +281,7 @@
}
},
"additionalProperties": false,
"required": ["glob", "input", "output"]
"required": ["glob", "input"]
},
{
"type": "string"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ export function assetPatterns(root: string, assets: AssetPatternClass[]) {
return assets.map((asset: AssetPatternClass, index: number): ObjectPattern => {
// Resolve input paths relative to workspace root and add slash at the end.
// eslint-disable-next-line prefer-const
let { input, output, ignore = [], glob } = asset;
let { input, output = '', ignore = [], glob } = asset;
input = path.resolve(root, input).replace(/\\/g, '/');
input = input.endsWith('/') ? input : input + '/';
output = output.endsWith('/') ? output : output + '/';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { statSync } from 'fs';
import assert from 'node:assert';
import * as path from 'path';
import { AssetPattern, AssetPatternClass } from '../builders/browser/schema';

Expand All @@ -21,7 +22,7 @@ export function normalizeAssetPatterns(
workspaceRoot: string,
projectRoot: string,
projectSourceRoot: string | undefined,
): AssetPatternClass[] {
): (AssetPatternClass & { output: string })[] {
if (assetPatterns.length === 0) {
return [];
}
Expand Down Expand Up @@ -67,13 +68,15 @@ export function normalizeAssetPatterns(

assetPattern = { glob, input, output };
} else {
assetPattern.output = path.join('.', assetPattern.output);
assetPattern.output = path.join('.', assetPattern.output ?? '');
}

assert(assetPattern.output !== undefined);

if (assetPattern.output.startsWith('..')) {
throw new Error('An asset cannot be written to a location outside of the output path.');
}

return assetPattern;
return assetPattern as AssetPatternClass & { output: string };
});
}
Empty file.
4 changes: 2 additions & 2 deletions packages/schematics/angular/application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ function addAppToWorkspaceFile(
polyfills: ['zone.js'],
tsConfig: `${projectRoot}tsconfig.app.json`,
inlineStyleLanguage,
assets: [`${sourceRoot}/favicon.ico`, `${sourceRoot}/assets`],
assets: [{ 'glob': '**/*', 'input': 'public' }],
styles: [`${sourceRoot}/styles.${options.style}`],
scripts: [],
},
Expand Down Expand Up @@ -285,7 +285,7 @@ function addAppToWorkspaceFile(
polyfills: ['zone.js', 'zone.js/testing'],
tsConfig: `${projectRoot}tsconfig.spec.json`,
inlineStyleLanguage,
assets: [`${sourceRoot}/favicon.ico`, `${sourceRoot}/assets`],
assets: [{ 'glob': '**/*', 'input': 'public' }],
styles: [`${sourceRoot}/styles.${options.style}`],
scripts: [],
},
Expand Down
12 changes: 6 additions & 6 deletions packages/schematics/angular/application/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('Application Schematic', () => {
jasmine.arrayContaining([
'/projects/foo/tsconfig.app.json',
'/projects/foo/tsconfig.spec.json',
'/projects/foo/src/favicon.ico',
'/projects/foo/public/favicon.ico',
'/projects/foo/src/index.html',
'/projects/foo/src/main.ts',
'/projects/foo/src/styles.css',
Expand Down Expand Up @@ -263,7 +263,7 @@ describe('Application Schematic', () => {
jasmine.arrayContaining([
'/tsconfig.app.json',
'/tsconfig.spec.json',
'/src/favicon.ico',
'/public/favicon.ico',
'/src/index.html',
'/src/main.ts',
'/src/styles.css',
Expand Down Expand Up @@ -448,7 +448,7 @@ describe('Application Schematic', () => {
expect(files).toEqual(
jasmine.arrayContaining([
'/projects/foo/tsconfig.app.json',
'/projects/foo/src/favicon.ico',
'/projects/foo/public/favicon.ico',
'/projects/foo/src/index.html',
'/projects/foo/src/main.ts',
'/projects/foo/src/styles.css',
Expand All @@ -473,7 +473,7 @@ describe('Application Schematic', () => {
expect(files).toEqual(
jasmine.arrayContaining([
'/projects/foo/tsconfig.app.json',
'/projects/foo/src/favicon.ico',
'/projects/foo/public/favicon.ico',
'/projects/foo/src/index.html',
'/projects/foo/src/main.ts',
'/projects/foo/src/styles.css',
Expand All @@ -499,7 +499,7 @@ describe('Application Schematic', () => {
expect(files).toEqual(
jasmine.arrayContaining([
'/projects/foo/tsconfig.app.json',
'/projects/foo/src/favicon.ico',
'/projects/foo/public/favicon.ico',
'/projects/foo/src/index.html',
'/projects/foo/src/main.ts',
'/projects/foo/src/styles.css',
Expand All @@ -519,7 +519,7 @@ describe('Application Schematic', () => {
jasmine.arrayContaining([
'/projects/foo/tsconfig.app.json',
'/projects/foo/tsconfig.spec.json',
'/projects/foo/src/favicon.ico',
'/projects/foo/public/favicon.ico',
'/projects/foo/src/index.html',
'/projects/foo/src/main.ts',
'/projects/foo/src/styles.css',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@
"updateMode": "prefetch",
"resources": {
"files": [
"/assets/**",
"<%= resourcesOutputPath %>/*.(svg|cur|jpg|jpeg|png|apng|webp|avif|gif|otf|ttf|woff|woff2)"
"/**/*.(svg|cur|jpg|jpeg|png|apng|webp|avif|gif|otf|ttf|woff|woff2)"
]
}
}
Expand Down
6 changes: 0 additions & 6 deletions packages/schematics/angular/service-worker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,10 @@ export default function (options: ServiceWorkerOptions): Rule {

const buildOptions = buildTarget.options as Record<string, string | boolean>;
let browserEntryPoint: string | undefined;
let resourcesOutputPath = '';
const ngswConfigPath = join(normalize(project.root), 'ngsw-config.json');

if (buildTarget.builder === Builders.Application) {
browserEntryPoint = buildOptions.browser as string;
resourcesOutputPath = '/media';
const productionConf = buildTarget.configurations?.production;
if (productionConf) {
productionConf.serviceWorker = ngswConfigPath;
Expand All @@ -134,9 +132,6 @@ export default function (options: ServiceWorkerOptions): Rule {
browserEntryPoint = buildOptions.main as string;
buildOptions.serviceWorker = true;
buildOptions.ngswConfigPath = ngswConfigPath;
if (buildOptions.resourcesOutputPath) {
resourcesOutputPath = normalize(`/${buildOptions.resourcesOutputPath}`);
}
}

await writeWorkspace(host, workspace);
Expand All @@ -147,7 +142,6 @@ export default function (options: ServiceWorkerOptions): Rule {
apply(url('./files'), [
applyTemplates({
...options,
resourcesOutputPath,
relativePathToWorkspaceRoot: relativePathToWorkspaceRoot(project.root),
}),
move(project.root),
Expand Down
9 changes: 0 additions & 9 deletions packages/schematics/angular/service-worker/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,6 @@ describe('Service Worker Schematic', () => {
expect(configNotInRoot.$schema).toBe(`../../${pathToNgswConfigSchema}`);
});

it('should add root assets RegExp', async () => {
const tree = await schematicRunner.runSchematic('service-worker', defaultOptions, appTree);
const pkgText = tree.readContent('/projects/bar/ngsw-config.json');
const config = JSON.parse(pkgText);
expect(config.assetGroups[1].resources.files).toContain(
'/media/*.(svg|cur|jpg|jpeg|png|apng|webp|avif|gif|otf|ttf|woff|woff2)',
);
});

it('should generate ngsw-config.json in root when the application is at root level', async () => {
const name = 'foo';
const rootAppOptions: ApplicationOptions = {
Expand Down

0 comments on commit 1399e7f

Please sign in to comment.