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

Angular apps not working in Safari v15 due to ReferenceError #24355

Closed
Meet1202 opened this issue Nov 30, 2022 · 9 comments · Fixed by #24357
Closed

Angular apps not working in Safari v15 due to ReferenceError #24355

Meet1202 opened this issue Nov 30, 2022 · 9 comments · Fixed by #24357
Assignees
Labels
P2 The issue is important to a large percentage of users, with a workaround
Milestone

Comments

@Meet1202
Copy link

Which @angular/* package(s) are the source of the bug?

Don't known / other

Is this a regression?

No

Description

When we open angular.io website in safari browser in mac then it giving blank page and if we do refresh that page than also it's giving blank page and when we open inspect element from developer open in safari browser and do refresh that page then it open perfectly so it's an issue I have provided screenshot.

  • This when I open angular.io link in Safari browser in Mac.
    Screenshot 2022-11-30 at 6 37 28 PM

  • This when I open inspect element of safari browser and still website is not working.
    Screenshot 2022-11-30 at 6 39 15 PM

  • And at last this after opening Inspect element I refresh the page then website is working properly.
    Screenshot 2022-11-30 at 6 39 27 PM

Please provide a link to a minimal reproduction of the bug

https://angular.io

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in (run ng version)

No response

Anything else?

No response

@kbrilla
Copy link

kbrilla commented Nov 30, 2022

Same hapenning on iOS 15.4. Don’t know about devtools though. But we had similar issue and It was connected to app being PWA - the issue that it did work when dev tools were open.

@VictorienTardif
Copy link

Hi, I discovered it was happening on iOS 15 only. Everything working fine on iOS 16.
Here is a screenshot of the console:
f4f6024e-687e-441d-ad14-3b0fe821a573

@matthewdenobrega
Copy link

I had this issue with my app - it wasn't loading on Safari 15 or in a Webview on iOS 15 (16 was fine) with a thrown exception about "Can't find variable t..." and an uninformative stack trace.

I tracked it down to forwardRefs - I was using forwardRefs on all my ControlValuAccessors, and removing these (they are unnecessary because they are in the class decorators) resolved this issue. This was across two apps - one Ionic and one Angular material.

So it looks like there's an issue with forwardRefs, at least when defining ControlValueAccessors, in Angular 15, on Safari 15 and iOS 15 webviews.

This was not an issue on Angular14, I only started seeing these errors after I upgraded.

@devversion
Copy link
Member

devversion commented Dec 1, 2022

Hi all, I've tracked down the root cause. It's a very subtle bug that got fixed in Safari v16. And yes, the bug doesn't reproduce when the devtools are opened. Here is how I tracked it down:

  1. Initially loaded the page with devtools closed, saw an error message like in Angular apps not working in Safari v15 due to ReferenceError #24355. The stack did not point to anything useful- especially since source maps can hide issues.
  2. Disabled source maps, closed dev-tools and re-freshed. Now the error looked like this:
    image
    • The error did not point to something useful either. Not even close to something with $o. Safari stack trace was incorrect.
  3. Rebuilt angular.io with optimization: false. Fortunately the issue was reproducable even with unoptimized build (making debugging easier):
    • image
  4. Now that the culprit JS code was finally visible- more investigation was possible.
    • The variable accessed is clearly in scope. See notification_component_c0.
    • There is some weird syntax for the static fields generated by the Angular compiler. e.g. notice static #_ = this.ecmp = X
  5. I initially thought the private identifier static fields are the problem, so I tried to understand where these are coming from:
    • Looked through Angular CLI's compilation pipeline and looked for Babel plugins.
    • There is a Babel plugin generating these for older browsers not supporting static {} blocks.
    • Surprisingly Angular compiler does not generate static {} blocks- but static fields.
    • Turned out TypeScript converts static fields to static blocks if useDefineForClassFields = false (which is the case in Angular v15 w/ target: 2022)
    • I've tried to isolate the problem and create a minimal reproduction, but couldn't reproduce. Since the issue was already very hard to track down (given devtools cannot being opened)- I've still assumed something odd could go on in Safari..
    • Looked through WebKit bug tracker and commits but couldn't find anything- except for private identifier methods fix in the parser— could have been related.
    • Ultimately I concluded that static private identifier fields are not the issue.
  6. I've tried to "tamper" the main bundle and modify parts to see what the issue could be, slowly removing parts of the code snippet in (3).
    • Tracked it down to "still" happen when there is code like (0, x).method() in the static field initializer. In other scenarios, the error quickly was gone- almost seeming like it's non-deterministic.
    • My assumption was that it's a scope tracking issue in the WebKit JS parsing.
    • Looked through Webkit commits and found a fix (in the reasonable time frame of Safari v15.6 -> v16)
    • Issue description seems very similar/matching: WebKit/WebKit@e8788a3
    • https://bugs.webkit.org/show_bug.cgi?id=236843
    • TypeScript will generate such expressions to ensure correct this context when it invokes methods. e.g. for animations
    • image

TL;DR: Angular emits valid JavaScript that is subject to a bug in Safari that got fixed in Safari v16. The issue does not reproduce when devtools are opened- and the Safari bug is related to scope tracking in the WebKit JS parsing. WebKit/WebKit@e8788a3. All Angular applications could be affected, but hard to notice/narrow down

We will look into downleveling static fields for Safari <16 so that Angular apps remain compatible with that browser, if selected via browserslist (or the default one).

@devversion devversion changed the title Angular.io website is not opening in Safari browser. Angular apps not working in Safari v15 due to ReferenceError Dec 1, 2022
@devversion devversion transferred this issue from angular/angular Dec 1, 2022
devversion added a commit to devversion/angular-cli that referenced this issue Dec 1, 2022
…argeting 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:
angular#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;
```
devversion added a commit to devversion/angular-cli that referenced this issue Dec 1, 2022
…argeting 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:
angular#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;
```

ds
@yjaaidi
Copy link

yjaaidi commented Dec 1, 2022

In my understanding, this happened specifically with this component because it's using animations + content projection.

  • Animations are causing the static block.
  • Content projection causes the static block to access the notification_component_c0 = ['*'] variable (outside the class) which breaks WebKit parser's scope analysis.
    ... but I might be totally wrong 🙃

The hardest thing here is to define what are other "similar" situations 😅

@devversion
Copy link
Member

devversion commented Dec 1, 2022

@yjaaidi Yes, very close. The static block is an artifact from Angular compiler adding the ecmp static field. TS with useDefineForClassFields=false then puts that into a static block. See: https://www.typescriptlang.org/play?target=9&useDefineForClassFields=true#code/MYGwhgzhAEAa0G8BQTpoPYGtoF5oBcAnAVwFMBuVNaCfMfAS2GlOAFsAHXAkiqtWvSYsAJg0LciZSkgC+QA

Then, yes, content projection results in a variable reference from a constant outside of the class. Together with animations which messes up the scope variable tracking- the issue surfaces.

devversion added a commit to devversion/angular-cli that referenced this issue Dec 1, 2022
…argeting 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:
angular#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;
```

note that in practice TypeScript with `useDefineForClassFields = false`
will put non-static members into the constructor as normal assignments
regardless- so there would be no change by the Babel plugin.
devversion added a commit to devversion/angular-cli that referenced this issue Dec 1, 2022
…argeting 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:
angular#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;
```

note that in practice TypeScript with `useDefineForClassFields = false`
will put non-static members into the constructor as normal assignments
regardless- so there would be no change by the Babel plugin.

Fixes angular#24355.
devversion added a commit to devversion/angular-cli that referenced this issue Dec 1, 2022
…argeting 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:
angular#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;
```

note that in practice TypeScript with `useDefineForClassFields = false`
will put non-static members into the constructor as normal assignments
regardless- so there would be no change by the Babel plugin.

Fixes angular#24355.
@albanx
Copy link

albanx commented Dec 1, 2022

Had a similar issue on Safari Mobile on the iphone. I was able to fix only by reset the browserlistrc to

> 1%
last 2 versions
not ie > 0
not ie_mob > 0
not dead

@muratcanoguzhan
Copy link

Had a similar issue on Safari Mobile on the iphone. I was able to fix only by reset the browserlistrc to

> 1%
last 2 versions
not ie > 0
not ie_mob > 0
not dead

While I was upgrading to Angular v15 browserlistsrc was deleted automatically.

angular-robot bot pushed a commit that referenced this issue Dec 2, 2022
…argeting 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;
```

note that in practice TypeScript with `useDefineForClassFields = false`
will put non-static members into the constructor as normal assignments
regardless- so there would be no change by the Babel plugin.

Fixes #24355.
angular-robot bot pushed a commit that referenced this issue Dec 2, 2022
…argeting 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;
```

note that in practice TypeScript with `useDefineForClassFields = false`
will put non-static members into the constructor as normal assignments
regardless- so there would be no change by the Babel plugin.

Fixes #24355.

(cherry picked from commit 25eaaa2)
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
9 participants