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
alan-agius4 committed Mar 25, 2024
1 parent 9d3aa46 commit 6530aa1
Show file tree
Hide file tree
Showing 32 changed files with 102 additions and 129 deletions.
2 changes: 1 addition & 1 deletion goldens/public-api/angular_devkit/build_angular/index.md
View file
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
0 ...r/pwa/pwa/files/root/manifest.webmanifest → ...pwa/pwa/files/assets/manifest.webmanifest
View file
File renamed without changes.
42 changes: 23 additions & 19 deletions packages/angular/pwa/pwa/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,23 +104,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 +129,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'), [template({ ...options }), move(assetsDir)])),
...[...indexFiles].map((path) => updateIndexFile(path)),
]);
};
Expand Down
19 changes: 4 additions & 15 deletions packages/angular/pwa/pwa/index_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('PWA Schematic', () => {

it('should create icon files', async () => {
const dimensions = [72, 96, 128, 144, 152, 192, 384, 512];
const iconPath = '/projects/bar/src/assets/icons/icon-';
const iconPath = '/projects/bar/public/icons/icon-';
const tree = await schematicRunner.runSchematic('ng-add', defaultOptions, appTree);

dimensions.forEach((d) => {
Expand All @@ -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
3 changes: 2 additions & 1 deletion packages/angular_devkit/build_angular/src/builders/application/schema.json
View file
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
3 changes: 2 additions & 1 deletion packages/angular_devkit/build_angular/src/builders/browser-esbuild/schema.json
View file
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
3 changes: 2 additions & 1 deletion packages/angular_devkit/build_angular/src/builders/browser/schema.json
View file
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
3 changes: 2 additions & 1 deletion packages/angular_devkit/build_angular/src/builders/karma/schema.json
View file
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
3 changes: 2 additions & 1 deletion packages/angular_devkit/build_angular/src/builders/server/schema.json
View file
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
3 changes: 2 additions & 1 deletion packages/angular_devkit/build_angular/src/builders/web-test-runner/schema.json
View file
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
2 changes: 1 addition & 1 deletion packages/angular_devkit/build_angular/src/tools/webpack/utils/helpers.ts
View file
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
9 changes: 6 additions & 3 deletions packages/angular_devkit/build_angular/src/utils/normalize-asset-patterns.ts
View file
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 };
});
}
0 ...les/common-files/src/favicon.ico.template → .../common-files/public/favicon.ico.template
View file
File renamed without changes.
Empty file removed packages/schematics/angular/application/files/common-files/src/assets/.gitkeep.template
View file
Empty file.
4 changes: 2 additions & 2 deletions packages/schematics/angular/application/index.ts
View file
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
View file
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
3 changes: 1 addition & 2 deletions packages/schematics/angular/service-worker/files/ngsw-config.json.template
View file
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
View file
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
Loading

1 comment on commit 6530aa1

@VBobCat
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update docs about ditching /assets;

Please sign in to comment.