Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove polyfills file #23938

Merged
merged 2 commits into from
Sep 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions goldens/public-api/angular_devkit/build_angular/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export interface BrowserBuilderOptions {
outputHashing?: OutputHashing;
outputPath: string;
poll?: number;
polyfills?: string;
polyfills?: Polyfills;
preserveSymlinks?: boolean;
progress?: boolean;
resourcesOutputPath?: string;
Expand Down Expand Up @@ -180,7 +180,7 @@ export interface KarmaBuilderOptions {
karmaConfig: string;
main: string;
poll?: number;
polyfills?: string;
polyfills?: Polyfills_2;
preserveSymlinks?: boolean;
progress?: boolean;
reporters?: string[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import { BuilderContext } from '@angular-devkit/architect';
import * as path from 'path';
import { normalizeAssetPatterns, normalizeOptimization, normalizeSourceMaps } from '../../utils';
import { normalizePolyfills } from '../../utils/normalize-polyfills';
import { Schema as BrowserBuilderOptions, OutputHashing } from '../browser/schema';

/**
Expand All @@ -33,10 +34,21 @@ export async function normalizeOptions(
workspaceRoot,
(projectMetadata.sourceRoot as string | undefined) ?? 'src',
);

// Normalize options
const mainEntryPoint = path.join(workspaceRoot, options.main);
const polyfillsEntryPoint = options.polyfills && path.join(workspaceRoot, options.polyfills);

// Currently esbuild do not support multiple files per entry-point
const [polyfillsEntryPoint, ...remainingPolyfills] = normalizePolyfills(
Copy link
Member

Choose a reason for hiding this comment

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

Can the path resolving step be skipped here? The absolute paths will end up in the sourcemaps.
esbuild should be able to resolve the relative paths directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As per off-line convo, lets do this in a follow up

options.polyfills,
workspaceRoot,
);

if (remainingPolyfills.length) {
context.logger.warn(
`The 'polyfills' option currently does not support multiple entries by this experimental builder. The first entry will be used.`,
);
}

const tsconfig = path.join(workspaceRoot, options.tsConfig);
const outputPath = path.join(workspaceRoot, options.outputPath);
const optimizationOptions = normalizeOptimization(options.optimization);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,22 @@
"description": "The full path for the main entry point to the app, relative to the current workspace."
},
"polyfills": {
"type": "string",
"description": "The full path for the polyfills file, relative to the current workspace."
"description": "Polyfills to be included in the build.",
"oneOf": [
{
"type": "array",
"description": "A list of polyfills to include in the build. Can be a full path for a file, relative to the current workspace or module specifier. Example: 'zone.js'.",
"items": {
"type": "string",
"uniqueItems": true
},
"default": []
},
{
"type": "string",
"description": "The full path for the polyfills file, relative to the current workspace or a module specifier. Example: 'zone.js'."
}
]
},
"tsConfig": {
"type": "string",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,22 @@
"description": "The full path for the main entry point to the app, relative to the current workspace."
},
"polyfills": {
"type": "string",
"description": "The full path for the polyfills file, relative to the current workspace."
"description": "Polyfills to be included in the build.",
"oneOf": [
{
"type": "array",
"description": "A list of polyfills to include in the build. Can be a full path for a file, relative to the current workspace or module specifier. Example: 'zone.js'.",
"items": {
"type": "string",
"uniqueItems": true
},
"default": []
},
{
"type": "string",
"description": "The full path for the polyfills file, relative to the current workspace or a module specifier. Example: 'zone.js'."
}
]
},
"tsConfig": {
"type": "string",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,16 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {

harness.expectFile('dist/polyfills.js').toNotExist();
});

it('resolves module specifiers in array', async () => {
harness.useTarget('build', {
...BASE_OPTIONS,
polyfills: ['zone.js', 'zone.js/testing'],
});

const { result } = await harness.executeOnce();
expect(result?.success).toBeTrue();
harness.expectFile('dist/polyfills.js').toExist();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,22 @@
"description": "The name of the Karma configuration file."
},
"polyfills": {
"type": "string",
"description": "The name of the polyfills file."
"description": "Polyfills to be included in the build.",
"oneOf": [
{
"type": "array",
"description": "A list of polyfills to include in the build. Can be a full path for a file, relative to the current workspace or module specifier. Example: 'zone.js'.",
"items": {
"type": "string",
"uniqueItems": true
},
"default": []
},
{
"type": "string",
"description": "The full path for the polyfills file, relative to the current workspace or a module specifier. Example: 'zone.js'."
}
]
},
"assets": {
"type": "array",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export interface BuildOptions {
statsJson: boolean;
hmr?: boolean;
main: string;
polyfills?: string;
polyfills: string[];
budgets: Budget[];
assets: AssetPatternClass[];
scripts: ScriptElement[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
normalizeFileReplacements,
} from './normalize-file-replacements';
import { NormalizedOptimizationOptions, normalizeOptimization } from './normalize-optimization';
import { normalizePolyfills } from './normalize-polyfills';
import { normalizeSourceMaps } from './normalize-source-maps';
import { getSupportedBrowsers } from './supported-browsers';

Expand All @@ -32,6 +33,7 @@ export type NormalizedBrowserBuilderSchema = BrowserBuilderSchema &
assets: AssetPatternClass[];
fileReplacements: NormalizedFileReplacement[];
optimization: NormalizedOptimizationOptions;
polyfills: string[];
};

export function normalizeBrowserSchema(
Expand All @@ -42,8 +44,6 @@ export function normalizeBrowserSchema(
metadata: json.JsonObject,
logger: logging.LoggerApi,
): NormalizedBrowserBuilderSchema {
const normalizedSourceMapOptions = normalizeSourceMaps(options.sourceMap || false);

return {
...options,
cache: normalizeCacheOptions(metadata, workspaceRoot),
Expand All @@ -55,7 +55,8 @@ export function normalizeBrowserSchema(
),
fileReplacements: normalizeFileReplacements(options.fileReplacements || [], workspaceRoot),
optimization: normalizeOptimization(options.optimization),
sourceMap: normalizedSourceMapOptions,
sourceMap: normalizeSourceMaps(options.sourceMap || false),
polyfills: normalizePolyfills(options.polyfills, workspaceRoot),
preserveSymlinks:
options.preserveSymlinks === undefined
? process.execArgv.includes('--preserve-symlinks')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import { existsSync } from 'fs';
import { resolve } from 'path';

export function normalizePolyfills(
polyfills: string[] | string | undefined,
root: string,
): string[] {
if (!polyfills) {
return [];
}

const polyfillsList = Array.isArray(polyfills) ? polyfills : [polyfills];

return polyfillsList.map((p) => {
const resolvedPath = resolve(root, p);

// If file doesn't exist, let the bundle resolve it using node module resolution.
return existsSync(resolvedPath) ? resolvedPath : p;
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise<Config
const isPlatformServer = buildOptions.platform === 'server';
const extraPlugins: { apply(compiler: Compiler): void }[] = [];
const extraRules: RuleSetRule[] = [];
const entryPoints: { [key: string]: [string, ...string[]] } = {};
const entryPoints: Configuration['entry'] = {};

// Load ESM `@angular/compiler-cli` using the TypeScript dynamic import workaround.
// Once TypeScript provides support for keeping the dynamic import this workaround can be
Expand Down Expand Up @@ -105,13 +105,32 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise<Config
extraPlugins.push(new ContextReplacementPlugin(/@?hapi|express[\\/]/));
}

if (!isPlatformServer) {
if (buildOptions.polyfills) {
const projectPolyfills = path.resolve(root, buildOptions.polyfills);
if (entryPoints['polyfills']) {
entryPoints['polyfills'].push(projectPolyfills);
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(...polyfillsExludingZoneTesting);
} else {
entryPoints['polyfills'] = polyfillsExludingZoneTesting;
}

if (polyfillsExludingZoneTesting.length !== polyfills.length) {
if (Array.isArray(entryPoints['main'])) {
entryPoints['main'].unshift(zoneTestingEntryPoint);
} else {
entryPoints['polyfills'] = [projectPolyfills];
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
Loading