-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
core: remove no-mutation-events audit & js callstack method #5509
Conversation
5ce0967
to
87905d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sgtm. can you file an issue for adding a violation to Chrome for use of mutation events?
you have a few more lines to remove in dbw_tester.js
done https://bugs.chromium.org/p/chromium/issues/detail?id=719009 |
pushed? |
87905d5
to
d08457c
Compare
pushed now What were your mutation event listener findings? :) |
@brendankenny you were 👍 on this, right? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like event-helpers
might be only used in the audit as well?
Also need to delete the EventListeners
artifact in artifacts.d.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise LGTM!
the things I can still learn exist 😮 |
Summary
no-mutation-events
audit is the only audit using a large chunk of heavy/slow gatherer code.If this becomes a priority again after passing our new audit process standards, then we still have this diff to look back on or it can become a Chrome violation to be picked up for free :)
comment from the future (paul in nov 2021): this PR also removes some
captureJSCallUsage()
weird stuff . it relied onError.prepareStackTrace
which is a wild lil API. We also found that some websites overwritewindow.Error
with something and this caused problems: #1194This PR removes all that so those websites can continue to be a menace but we no longer care.