Skip to content

Commit

Permalink
feat(@schematics/angular): drop polyfills.ts file from new templates
Browse files Browse the repository at this point in the history
With this change we drop the `polyfills.ts` from new application projects and add the polyfills directly in the `angular.json`. This is possible as now the `polyfills` option accept an array of module specifiers.

This change also fixes another open issue (#14432) which was caused by the missing polyfills file in the library test setup.

Closes #14432
  • Loading branch information
alan-agius4 committed Sep 26, 2022
1 parent c592ec5 commit 597bfea
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,33 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise<Config
extraPlugins.push(new ContextReplacementPlugin(/@?hapi|express[\\/]/));
}

if (buildOptions.polyfills.length) {
if (polyfills?.length) {
// `zone.js/testing` is a **special** polyfill because when not imported in the main it fails with the below errors:
// `Error: Expected to be running in 'ProxyZone', but it was not found.`
// This was also the reason why previously it was imported in `test.ts` as the first module.
// From Jia li:
// This is because the jasmine functions such as beforeEach/it will not be patched by zone.js since
// jasmine will not be loaded yet, so the ProxyZone will not be there. We have to load zone-testing.js after
// jasmine is ready.
// We could force loading 'zone.js/testing' prior to jasmine by changing the order of scripts in 'karma-context.html'.
// But this has it's own problems as zone.js needs to be loaded prior to jasmine due to patching of timing functions
// See: https://github.com/jasmine/jasmine/issues/1944
// Thus the correct order is zone.js -> jasmine -> zone.js/testing.
const zoneTestingEntryPoint = 'zone.js/testing';
const polyfillsExludingZoneTesting = polyfills.filter((p) => p !== zoneTestingEntryPoint);

if (Array.isArray(entryPoints['polyfills'])) {
entryPoints['polyfills'].push(...buildOptions.polyfills);
entryPoints['polyfills'].push(...polyfillsExludingZoneTesting);
} else {
entryPoints['polyfills'] = buildOptions.polyfills;
entryPoints['polyfills'] = polyfillsExludingZoneTesting;
}

if (polyfillsExludingZoneTesting.length !== polyfills.length) {
if (Array.isArray(entryPoints['main'])) {
entryPoints['main'].unshift(zoneTestingEntryPoint);
} else {
entryPoints['main'] = [zoneTestingEntryPoint, entryPoints['main'] as string];
}
}
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// This file is required by karma.conf.js and loads recursively all the .spec and framework files

import 'zone.js/testing';
import { getTestBed } from '@angular/core/testing';
import {
BrowserDynamicTestingModule,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
"types": []
},
"files": [
"src/main.ts",
"src/polyfills.ts"
"src/main.ts"
],
"include": [
"src/**/*.d.ts"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
]
},
"files": [
"src/test.ts",
"src/polyfills.ts"
"src/test.ts"
],
"include": [
"src/**/*.spec.ts",
Expand Down
4 changes: 2 additions & 2 deletions packages/schematics/angular/application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ function addAppToWorkspaceFile(
outputPath: `dist/${folderName}`,
index: `${sourceRoot}/index.html`,
main: `${sourceRoot}/main.ts`,
polyfills: `${sourceRoot}/polyfills.ts`,
polyfills: ['zone.js'],
tsConfig: `${projectRoot}tsconfig.app.json`,
inlineStyleLanguage,
assets: [`${sourceRoot}/favicon.ico`, `${sourceRoot}/assets`],
Expand Down Expand Up @@ -211,7 +211,7 @@ function addAppToWorkspaceFile(
builder: Builders.Karma,
options: {
main: `${sourceRoot}/test.ts`,
polyfills: `${sourceRoot}/polyfills.ts`,
polyfills: ['zone.js', 'zone.js/testing'],
tsConfig: `${projectRoot}tsconfig.spec.json`,
karmaConfig: `${projectRoot}karma.conf.js`,
inlineStyleLanguage,
Expand Down
15 changes: 5 additions & 10 deletions packages/schematics/angular/application/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ describe('Application Schematic', () => {
'/projects/foo/src/favicon.ico',
'/projects/foo/src/index.html',
'/projects/foo/src/main.ts',
'/projects/foo/src/polyfills.ts',
'/projects/foo/src/styles.css',
'/projects/foo/src/test.ts',
'/projects/foo/src/app/app.module.ts',
Expand Down Expand Up @@ -157,7 +156,7 @@ describe('Application Schematic', () => {
.runSchematicAsync('application', defaultOptions, workspaceTree)
.toPromise();
const { files, extends: _extends } = readJsonFile(tree, '/projects/foo/tsconfig.app.json');
expect(files).toEqual(['src/main.ts', 'src/polyfills.ts']);
expect(files).toEqual(['src/main.ts']);
expect(_extends).toBe('../../tsconfig.json');
});

Expand All @@ -166,7 +165,7 @@ describe('Application Schematic', () => {
.runSchematicAsync('application', defaultOptions, workspaceTree)
.toPromise();
const { files, extends: _extends } = readJsonFile(tree, '/projects/foo/tsconfig.spec.json');
expect(files).toEqual(['src/test.ts', 'src/polyfills.ts']);
expect(files).toEqual(['src/test.ts']);
expect(_extends).toBe('../../tsconfig.json');
});

Expand Down Expand Up @@ -270,7 +269,6 @@ describe('Application Schematic', () => {
'/projects/foo/src/favicon.ico',
'/projects/foo/src/index.html',
'/projects/foo/src/main.ts',
'/projects/foo/src/polyfills.ts',
'/projects/foo/src/styles.css',
'/projects/foo/src/app/app.module.ts',
'/projects/foo/src/app/app.component.ts',
Expand Down Expand Up @@ -300,7 +298,6 @@ describe('Application Schematic', () => {
'/projects/foo/src/favicon.ico',
'/projects/foo/src/index.html',
'/projects/foo/src/main.ts',
'/projects/foo/src/polyfills.ts',
'/projects/foo/src/styles.css',
'/projects/foo/src/app/app.module.ts',
'/projects/foo/src/app/app.component.css',
Expand Down Expand Up @@ -331,7 +328,6 @@ describe('Application Schematic', () => {
'/projects/foo/src/favicon.ico',
'/projects/foo/src/index.html',
'/projects/foo/src/main.ts',
'/projects/foo/src/polyfills.ts',
'/projects/foo/src/styles.css',
'/projects/foo/src/app/app.module.ts',
'/projects/foo/src/app/app.component.html',
Expand Down Expand Up @@ -413,7 +409,6 @@ describe('Application Schematic', () => {
'/src/favicon.ico',
'/src/index.html',
'/src/main.ts',
'/src/polyfills.ts',
'/src/styles.css',
'/src/test.ts',
'/src/app/app.module.ts',
Expand All @@ -437,7 +432,7 @@ describe('Application Schematic', () => {
const buildOpt = prj.architect.build.options;
expect(buildOpt.index).toEqual('src/index.html');
expect(buildOpt.main).toEqual('src/main.ts');
expect(buildOpt.polyfills).toEqual('src/polyfills.ts');
expect(buildOpt.polyfills).toEqual(['zone.js']);
expect(buildOpt.tsConfig).toEqual('tsconfig.app.json');

const testOpt = prj.architect.test.options;
Expand Down Expand Up @@ -515,7 +510,7 @@ describe('Application Schematic', () => {
expect(appTsConfig.extends).toEqual('./tsconfig.json');
const specTsConfig = readJsonFile(tree, '/tsconfig.spec.json');
expect(specTsConfig.extends).toEqual('./tsconfig.json');
expect(specTsConfig.files).toEqual(['src/test.ts', 'src/polyfills.ts']);
expect(specTsConfig.files).toEqual(['src/test.ts']);
});

it(`should create correct paths when 'newProjectRoot' is blank`, async () => {
Expand All @@ -532,7 +527,7 @@ describe('Application Schematic', () => {
const buildOpt = project.architect.build.options;
expect(buildOpt.index).toEqual('foo/src/index.html');
expect(buildOpt.main).toEqual('foo/src/main.ts');
expect(buildOpt.polyfills).toEqual('foo/src/polyfills.ts');
expect(buildOpt.polyfills).toEqual(['zone.js']);
expect(buildOpt.tsConfig).toEqual('foo/tsconfig.app.json');

const appTsConfig = readJsonFile(tree, '/foo/tsconfig.app.json');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
// This file is required by karma.conf.js and loads recursively all the .spec and framework files

import 'zone.js';
import 'zone.js/testing';
import { getTestBed } from '@angular/core/testing';
import {
BrowserDynamicTestingModule,
Expand Down
1 change: 1 addition & 0 deletions packages/schematics/angular/library/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ function addLibToWorkspaceFile(
options: {
main: `${projectRoot}/src/test.ts`,
tsConfig: `${projectRoot}/tsconfig.spec.json`,
polyfills: ['zone.js', 'zone.js/testing'],
karmaConfig: `${projectRoot}/karma.conf.js`,
},
},
Expand Down
1 change: 0 additions & 1 deletion tests/legacy-cli/e2e/tests/build/disk-cache-purge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export default async function () {

// No need to include all applications code to verify disk cache existence.
await writeFile('src/main.ts', 'console.log(1);');
await writeFile('src/polyfills.ts', 'console.log(1);');

// Enable cache for all environments
await updateJsonFile('angular.json', (config) => {
Expand Down
1 change: 0 additions & 1 deletion tests/legacy-cli/e2e/tests/build/disk-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export default async function () {

// No need to include all applications code to verify disk cache existence.
await writeFile('src/main.ts', 'console.log(1);');
await writeFile('src/polyfills.ts', 'console.log(1);');

try {
// Should be enabled by default.
Expand Down

0 comments on commit 597bfea

Please sign in to comment.