-
Notifications
You must be signed in to change notification settings - Fork 407
Conversation
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.
This looks great. I will have to do some perf testing before I merge it in. (Can you take care of the minor comments?)
lib/zone.ts
Outdated
// Cause the error to extract the stack frames. | ||
detectZone.runTask(detectZone.scheduleMacroTask('detect', detectRunFn, null, () => null, null)); | ||
}); | ||
Error.stackTraceLimit = 15; |
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.
we should store and restore, rather than reset to 15
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.
Sure, will do.
lib/zone.ts
Outdated
const fakeTransitionTo = | ||
(toState: TaskState, fromState1: TaskState, fromState2: TaskState) => {}; | ||
childDetectZone.scheduleEventTask( | ||
'detect', |
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.
to keep the size down, can we reuse existing string such as blacklistedStackFramesSymbol
?
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.
sure, I will modify this one.
65e8f5d
to
aec8cff
Compare
* feat(error): an idea for simple stack * merge detectZone
An idea to only throw one error to detect most zone stack trace frames.