Skip to content

fix: angular detection can be less blunt#1687

Merged
pauldambra merged 1 commit intomainfrom
fix/angular-detection
Jan 24, 2025
Merged

fix: angular detection can be less blunt#1687
pauldambra merged 1 commit intomainfrom
fix/angular-detection

Conversation

@pauldambra
Copy link
Member

from investigation of how rrweb has done this over time and how safari behaves with different versions manually patched

i learned that Angular Zone advertises its unpatched versions in a known/discoverable location

let's just use them instead of going to an iframe when they're present

@vercel
Copy link

vercel bot commented Jan 24, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Jan 24, 2025 9:53am

@pauldambra
Copy link
Member Author

PostHog/posthog-rrweb#24

+*/
+function angularZoneUnpatchedAlternative$1(key) {
+ const angularUnpatchedVersionSymbol = (
+ globalThis
Copy link
Member Author

Choose a reason for hiding this comment

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

angular puts a function on Zone you can call with the name of the thing
and you get back undefined or where it has stored the thing

+ angularUnpatchedVersionSymbol &&
+ (globalThis)[angularUnpatchedVersionSymbol]
+ ) {
+ return (globalThis)[
Copy link
Member Author

Choose a reason for hiding this comment

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

so, if there's an unpatched version we can use that

(confirmed this returns expected values on customer site)

if (untaintedBasePrototype$1[key])
return untaintedBasePrototype$1[key];
- const defaultObj = globalThis[key];
+ const defaultObj = angularZoneUnpatchedAlternative$1(key) || globalThis[key];
Copy link
Member Author

Choose a reason for hiding this comment

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

so we use the angular one if its present or the default global
even if this was an expensive lookup (it's not) the results are cached

+You can rename Zone, but this is a good enough proxy to avoid going to an iframe to get the untainted versions.
+see: https://github.com/angular/angular/issues/26948
+*/
+function angularZoneUnpatchedAlternative(key) {
Copy link
Member Author

Choose a reason for hiding this comment

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

same here because we're patching a bundle that is tree-shaken badly

)
);
- if (isUntaintedAccessors && isUntaintedMethods && !isAngularZonePresent$1()) {
+ if (isUntaintedAccessors && isUntaintedMethods) {
Copy link
Member Author

Choose a reason for hiding this comment

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

and so no angular check here

@github-actions
Copy link
Contributor

Size Change: +2.47 kB (+0.08%)

Total Size: 3.28 MB

Filename Size Change
dist/all-external-dependencies.js 209 kB +354 B (+0.17%)
dist/array.full.js 370 kB +354 B (+0.1%)
dist/array.full.no-external.js 369 kB +354 B (+0.1%)
dist/module.full.js 370 kB +354 B (+0.1%)
dist/module.full.no-external.js 369 kB +354 B (+0.1%)
dist/recorder-v2.js 117 kB +350 B (+0.3%)
dist/recorder.js 117 kB +350 B (+0.3%)
ℹ️ View Unchanged
Filename Size
dist/array.full.es5.js 266 kB
dist/array.js 182 kB
dist/array.no-external.js 180 kB
dist/customizations.full.js 13.8 kB
dist/dead-clicks-autocapture.js 14.4 kB
dist/exception-autocapture.js 9.48 kB
dist/external-scripts-loader.js 2.64 kB
dist/main.js 182 kB
dist/module.js 182 kB
dist/module.no-external.js 180 kB
dist/surveys-preview.js 67.5 kB
dist/surveys.js 64.2 kB
dist/tracing-headers.js 1.76 kB
dist/web-vitals.js 10.4 kB

compressed-size-action

Copy link
Contributor

@daibhin daibhin left a comment

Choose a reason for hiding this comment

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

Feels like it couldn't hurt 😅 Given this is an Angular specific issue I think it's relatively safe to test it out

};
+/*
+Angular zone patches many things and can pass the untainted checks below, causing performance issues
+Angular zone, puts the unpatched originals on the window, and the names for hose on the zone object.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
+Angular zone, puts the unpatched originals on the window, and the names for hose on the zone object.
+Angular zone, puts the unpatched originals on the window, and the names for those on the zone object.

@pauldambra pauldambra merged commit 61480e3 into main Jan 24, 2025
31 checks passed
@pauldambra pauldambra deleted the fix/angular-detection branch January 24, 2025 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants