Multi-instance <TimeToInitialDisplay> / <TimeToFullDisplay> coordination; a multi-signal TTID/TTFD system#6090
Multi-instance <TimeToInitialDisplay> / <TimeToFullDisplay> coordination; a multi-signal TTID/TTFD system#6090
<TimeToInitialDisplay> / <TimeToFullDisplay> coordination; a multi-signal TTID/TTFD system#6090Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
alwx
force-pushed
the
alwx/enhancement/multiple-ttid-ttfd
branch
from
97bd1be to
cb268c8
Compare
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
|
Moved it back to draft because multi-instance coordination needs to be handled more carefully. |
| useEffect(() => { | ||
| if (!parentSpanId || !useRegistry) { | ||
| return undefined; | ||
| } | ||
| return registerCheckpoint(kind, parentSpanId, checkpointId, localReady); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [kind, parentSpanId, useRegistry, checkpointId]); |
There was a problem hiding this comment.
Unmounting a not-ready checkpoint can flip aggregate to true, causing premature TTID/TTFD
When a registry-mode component unmounts, the cleanup at line 199 calls the unregister returned by registerCheckpoint, which deletes the checkpoint and calls reevaluate. If the unmounting checkpoint was the only not-ready one, removing it flips the aggregate to true, immediately notifying peers that 'all are ready' even though that source never actually became ready. A conditionally-rendered loading section that unmounts before its data resolves will incorrectly satisfy the coordination, recording an incomplete display.
Verification
Read registerCheckpoint's returned unregister fn in timeToDisplayCoordinator.ts (lines 93-102): it deletes the checkpoint then calls reevaluate(e). computeAggregate returns true when all remaining checkpoints are ready; thus removing the only not-ready entry flips the cached aggregate to true and notifies listeners. No guard exists for this scenario.
Identified by Warden find-bugs · GW4-XFB
1 participant
📢 Type of change
📜 Description
Somthing that was requested by our customers: when a screen has multiple async data sources, you can now mount one
<TimeToFullDisplay>per source — the TTID/TTFD will get recorded only when all the sources reportready. That makes it possible to use TTID/TTFD when the screen comes with a bunch of individual components to handle not so simple real-world scenarios.For example, a screen might have:
All of these load independently, at different times, and the screen isn't actually fully displayed until every one of them has resolved and rendered. With the current SDK, the only way to handle this is to hoist state management above all of these components, track which ones have finished loading, and only fire the TTFD signal once everything reports in. That's a significant amount of orchestration code that really belongs in the SDK itself.
What we're doing here is basically making it work with multiple TTID/TTFD components to handle multiple signals coming from different sources.
The docs will be updated soon.
💚 How did you test it?
Tests were added.
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps