feat(nbstore): new doc sync engine #8918
Conversation
Your org has enabled the Graphite merge queue for merging into canaryAdd the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
graphite-app
bot
requested review from
CatsJuice,
forehalo,
JimmFly and
pengx17
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| waitForConnected(signal?: AbortSignal) { | ||
| return new Promise<void>((resolve, reject) => { | ||
| if (this.status === 'connected') { | ||
| resolve(); | ||
| return; | ||
| } | ||
|
|
||
| this.onStatusChanged(status => { | ||
| if (status === 'connected') { | ||
| resolve(); | ||
| } | ||
| }); | ||
|
|
||
| signal?.addEventListener('abort', reason => { | ||
| reject(reason); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The AbortSignal event listener should be cleaned up if the connection is already connected. Currently, if this.status === 'connected', the promise resolves but the abort listener is still registered. Consider returning a cleanup function from addEventListener and calling it in the early return case:
const cleanup = signal?.addEventListener('abort', reason => {
reject(reason);
});
if (this.status === 'connected') {
cleanup?.();
resolve();
return;
}Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
EYHN
changed the title
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 43f079e. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
EYHN
force-pushed
the
eyhn/feat/nb-sync
branch
from
aa5afd5
to
ca920c9
Compare
EYHN
force-pushed
the
eyhn/feat/nb-sync
branch
from
ca920c9
to
43f079e
Compare
No description provided.