Skip to content

Commit b19319d

Browse files
committed
fix(@angular-devkit/build-angular): downlevel class properties when targeting Safari <v15
The Angular compiler is dependent on static fields being attached to user-defined classes. e.g. `static ecmp = defineComponent`. These static fields sometimes rely on variables from outside of the class. e.g. the Angular compiler generates constants for content projection that are then accessed in the static field initializer. Surprisingly such access to these variables may break in Safari <v15 when a page is loaded without devtools open. The bug (already solved in v16 of Safari)- is very subtle, hard to re-reproduce but basically variable scope tracking is broken. This bug is triggered by additional parenthesis in the initializer expression. See: https://bugs.webkit.org/show_bug.cgi?id=236843. The TypeScript compiler may generate such additional parenthesis when it tries to adjust the `this` context when invoking methods, such as for defining animations in the `ecmp` definition. More details can be found here: #24355 (comment) To ensure Angular applications are not subject to this bug when targeting Safari <v15. v15 Safari, both for iOS and Mac is still part of the default CLI browserslist with `last 2 Safari majors` (at time of writing). Note that it is important that the Babel plugin properly handles the downleveling of static block-defined members. TypeScript will transform static fields, like `static ecmp` into `static { this.ecmp = X }` when `useDefineForClassFields = false` (which is the case for CLI apps). The class properties plugin from Babel seems to handle this in an acceptable way. Unlike actual static fields, Babel will not use helpers like `defineProperty` for such extracted static blocks though. e.g. See repro: https://gist.github.com/devversion/dec0dea26e348c509921bf62079b60be ```js class Test { x = true; static b = true; static { this.a = true; } } // into class X { constructor() { _defineProperty(this, "x", true); } } _defineProperty(X, "b", true); X.a = true; ```
1 parent 5ca7317 commit b19319d

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

packages/angular_devkit/build_angular/src/babel/presets/application.ts

+21
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@ import type {
1616
import { strict as assert } from 'assert';
1717
import * as fs from 'fs';
1818
import * as path from 'path';
19+
import browserslist from 'browserslist';
20+
21+
/**
22+
* List of browsers which are affected by a WebKit bug where class field
23+
* initializers might have incorrect variable scopes.
24+
*
25+
* See: https://github.com/angular/angular-cli/issues/24355#issuecomment-1333477033
26+
* See: https://github.com/WebKit/WebKit/commit/e8788a34b3d5f5b4edd7ff6450b80936bff396f2
27+
*/
28+
const safariClassFieldScopeBugBrowsers = browserslist(['Safari <15', 'iOS <15']);
1929

2030
export type DiagnosticReporter = (type: 'error' | 'warning' | 'info', message: string) => void;
2131

@@ -172,12 +182,23 @@ export default function (api: unknown, options: ApplicationPresetOptions) {
172182
}
173183

174184
if (options.forcePresetEnv) {
185+
const selectedBrowsers = browserslist(options.supportedBrowsers);
186+
const includePlugins: string[] = [];
187+
188+
// If a Safari browser affected by the class field scope bug is selected, we
189+
// downlevel class properties by ensuring the class properties Babel plugin
190+
// is always included- regardless of the preset-env targets.
191+
if (safariClassFieldScopeBugBrowsers.some((b) => selectedBrowsers.includes(b))) {
192+
includePlugins.push('@babel/plugin-proposal-class-properties');
193+
}
194+
175195
presets.push([
176196
require('@babel/preset-env').default,
177197
{
178198
bugfixes: true,
179199
modules: false,
180200
targets: options.supportedBrowsers,
201+
include: includePlugins,
181202
exclude: ['transform-typeof-symbol'],
182203
},
183204
]);

packages/angular_devkit/build_angular/src/babel/webpack-loader.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ export default custom<ApplicationPresetOptions>(() => {
108108
// Analyze for ES target processing
109109
if (customOptions.supportedBrowsers?.length) {
110110
// Applications code ES version can be controlled using TypeScript's `target` option.
111-
// However, this doesn't effect libraries and hence we use preset-env to downlevel ES fetaures
111+
// However, this doesn't effect libraries and hence we use preset-env to downlevel ES features
112112
// based on the supported browsers in browserlist.
113113
customOptions.forcePresetEnv = true;
114114
}

0 commit comments

Comments
 (0)