angular / angular-cli

CLI tool for Angular
https://cli.angular.io
MIT License
26.74k stars 11.98k forks source link

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

Closed Meet1202 closed 1 year ago

Meet1202 commented 1 year ago

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.

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 https://github.com/angular/angular-cli/issues/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: https://github.com/WebKit/WebKit/commit/e8788a34b3d5f5b4edd7ff6450b80936bff396f2
    • 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. https://github.com/WebKit/WebKit/commit/e8788a34b3d5f5b4edd7ff6450b80936bff396f2. 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).

yjaaidi commented 1 year ago

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

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

devversion commented 1 year ago

@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.

albanx commented 1 year ago

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 commented 1 year ago

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-automatic-lock-bot[bot] commented 1 year ago

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.