docs(frame): state the portal-side contract for closing a slider app - #449
Merged
Merged
Conversation
The SDK half of `closeApplication` works: it posts the command and stops. Everything after — the animation, the focus trap, removing the frame — is the portal's, and an application author cannot reach any of it from inside the iframe. So when a close misbehaves, our documentation is where they look, and ours said "Closes the application slider." and nothing else. Three consequences now stated where the method is documented. The promise is not a completion signal. It is sent with `isSafely: false`, so nothing settles it but the portal's answer — and on some builds that answer never comes, because an exception raised while closing fires before the response `postMessage` (#328), which makes `await closeApplication()` wait forever. Cleanup therefore belongs before the call, never after it. Our own frame-app skeleton was teaching the broken shape — `await $b24.parent.closeApplication()` as the last line of a handler — as was the frame-ui skill. Both fixed, and the skill gained the gotcha. The JSDoc on both methods carried a `@memo` justifying `isSafely: false`: "everything will be closed, and timeout will not be able to do anything". #328 is the case that reasoning does not cover — when the portal throws, the frame is not destroyed and the promise is simply stranded. The note now says so, and leaves the switch to `isSafely: true` open rather than implying it was settled. Refs #328 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F22e2ft66y7nuBJjzdThBr
Two corrections from review. The slider does close — the report says so; what survives is a zero-width iframe and, more to the point, `inert` left on the **parent page**, which is why a reload is needed. The note read as though the slider stayed up. And it was three times longer than the fact it carried. Cut to two bullets and a warning; the JSDoc note likewise. Dropped the speculation about switching to `isSafely: true` — that is a decision, not documentation. Refs #328 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F22e2ft66y7nuBJjzdThBr
Same content, told as an example. The prose said three times what one snippet shows: cleanup first, close without awaiting, `.catch()` on the way out. The warning about the portal-side defect stays as prose — that one cannot be shown by example. Both methods gained an `@example`, which the #439 gate promptly rejected for calling an undeclared `saveDraft()`. Replaced with a real SDK call. Refs #328 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F22e2ft66y7nuBJjzdThBr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #328 — the documentation half, which a comment on that issue argued is not optional.
Why documentation is our half of a platform bug
The SDK's part of
closeApplicationworks: it posts the command to the parent window and stops. Everything after — animating the slider shut, releasing the focus trap, removing the frame — is the portal's, and an application author cannot reach any of it from inside the iframe. So when a close misbehaves, our documentation is where they look first.Ours said, in full: "Closes the application slider."
Same shape as #356/#357, where the SDK's half was correct, the failure was entirely framework-side, and the documentation still had to change — because it could not tell the developer their half had succeeded.
What is now stated
At
parent.closeApplication, withslider.closeSliderAppPagepointing at it:isSafely: false, so only the portal's answer settles it — and on affected builds that answer never comes, because the exception fires before the responsepostMessage.await $b24.parent.closeApplication()then waits forever. Cleanup belongs before the call, never after it.inertdefect, in a warning block: what the reader is actually seeing when the portal freezes after a close, that a page reload is the only recovery, that it is reported and handed to the core team, and that it is not a bug in their application.We were teaching the broken shape
Two of our own files told readers to do the thing that hangs:
99.examples/2.frame-app-skeleton.mdasync function close() { await $b24?.parent.closeApplication() }skills/b24jssdk-frame-ui/SKILL.mdawait $b24.parent.closeApplication()Both now fire-and-forget with a
.catch(() => {})and a comment saying why. The skill gained the gotcha line; it already warned about the unhandled rejection atdestroy(), but not about theawaitthat never returns.The JSDoc
@memowas falsifiedBoth methods carried this justification for
isSafely: false:#328 is precisely the case that reasoning does not cover. When the portal throws, the frame is not destroyed — the report shows the closed slider's iframe still in the DOM at zero width — so there is no "everything will be closed", and the promise is simply stranded. The note now says so, and leaves the switch to
isSafely: trueexplicitly open rather than implying it was settled.Not in this PR
The
isSafely: truemitigation itself. It changes runtime behaviour of a public method, so it wants the full review panel and a maintainer's decision — raised separately.Verification
docs-typecheck-blocks(158),skills:typecheck-blocks(83),jsdoc:typecheck-blocks(41) andpackage-jssdk:typecheckall clean;docs-lint --strict,md-internal-linksandlint:mdclean; 121 script tests green. Theaudited:stamps on the two touched pages moved to today, since their cited sources changed.The docs gate caught my own snippet using an undeclared
saveDraft()— worth noting as the gate doing its job on this very PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01F22e2ft66y7nuBJjzdThBr
Generated by Claude Code