ref(react): Use new span API in React Profiler #10104
Conversation
AbhiPrasad
requested review from
a team,
anonrig and
ale-cota
and removed request for
a team
AbhiPrasad
changed the title
| @@ -1,8 +1,5 @@ | |||
| /* eslint-disable @typescript-eslint/no-unsafe-member-access */ | |||
packages/react/src/profiler.tsx
Outdated
| description: `<${name}>`, | ||
| const activeSpan = getActiveSpan(); | ||
| if (activeSpan) { | ||
| this._mountSpan = startInactiveSpan({ |
There was a problem hiding this comment.
just thinking, do we need to check here if _mountSpan (same below for _updateSpan) is already set, to ensure we clean up the span? 🤔 or are we fine just overwriting the reference?
There was a problem hiding this comment.
Yup we should do a check - 934da39
packages/react/src/profiler.tsx
Outdated
| name: `<${this.props.name}>`, | ||
| op: REACT_UPDATE_OP, | ||
| origin: 'auto.ui.react.profiler', | ||
| startTimestamp: now, | ||
| data: { |
There was a problem hiding this comment.
l: while we at it, we should also rename this to attributes everywhere! We can also do this in a follow up, though.
There was a problem hiding this comment.
Done with c517e92, will keep it in mind for the future.
packages/react/src/profiler.tsx
Outdated
| op: REACT_UPDATE_OP, | ||
| origin: 'auto.ui.react.profiler', | ||
| startTimestamp: now, | ||
| parentSpanId: this._mountSpan.spanContext().spanId, |
There was a problem hiding this comment.
this will not work anymore going forward - parentSpanId is/will be deprecated. Instead, you need to pass a scope that has the correct span as active span! 😬 But you can basically do this:
const scope = new Scope();
scope.setSpan(this._mountSpan);
startInactiveSpan({ scope, ... });There was a problem hiding this comment.
dang, good to know. Fixed with 64a14b6
There was a problem hiding this comment.
Ahh scope.setSpan is deprecated 😢
Any other way we can enforce the parent child relationship here?
There was a problem hiding this comment.
The way to go is #10195, which Luca just added for exactly this case :D
AbhiPrasad
force-pushed
the
abhi-react-profiler-deprecate
branch
from
fa10cf6 to
ef7a534
Compare
size-limit report 📦
|
ref #10100