[Preview NG] Add functions to sanitize objects for sending to Preview#3492
[Preview NG] Add functions to sanitize objects for sending to Preview#3492jeremywiebe merged 3 commits intomainfrom
Conversation
🗄️ Schema Change: No Changes ✅ |
🛠️ Item Splitting: No Changes ✅ |
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (339d4e6) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3492If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3492If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3492 |
jeremywiebe
changed the title
github-actions
bot
added
item-splitting-change
schema-change
|
Size Change: 0 B Total Size: 499 kB ℹ️ View Unchanged
|
jeremywiebe
changed the base branch from
jer/preview-ng-3-message-types
to
main
jeremywiebe
force-pushed
the
jer/preview-ng-4-sanitize-api-options
branch
from
8bc23c9 to
25f974c
Compare
jeremywiebe
changed the title
| * carries its own apiOptions). This will be simplified to a single apiOptions | ||
| * when we restructure the preview data types. | ||
| */ | ||
| export function sanitizePreviewData( |
There was a problem hiding this comment.
The preview system needs to send API options into the preview panel because some API Options can affect behaviour/rendering. But, some API Options are not serializable and deal with host integration (for example, callbacks) and so we need to sanitize the options before sending them over the "bridge".
| imagePreloader: _____, | ||
| trackInteraction: ______, | ||
| setDrawingAreaAvailable: ________, | ||
| baseElements: _________, |
There was a problem hiding this comment.
This is one key that can affect rendering. It is used in some contexts in the frontend application, but since this contains React elements, we can't send it across the bridge. In reality, host applications only set the Link element within this key and generally use it to override the behaviour of handling the clicked link vs. actually changing how it renders.
jeremywiebe
requested review from
a team,
catandthemachines and
mark-fitzgerald
github-actions
bot
added
schema-change
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
github-actions
bot
removed
schema-change
…ions] Add sanitizePreviewData and sanitizeApiOptions with tests
jeremywiebe
force-pushed
the
jer/preview-ng-4-sanitize-api-options
branch
from
d534a50 to
0419ad1
Compare
| const result = sanitizePreviewData(previewContent); | ||
|
|
||
| expect(result.type).toBe("question"); | ||
| if (result.type === "question") { |
There was a problem hiding this comment.
Is this conditional needed? If result.type does not equal "question", then the assertion before it will fail and cause the test to exit, correct? Is this here for some TypeScript reason?
There was a problem hiding this comment.
This is needed because the previous assertion doesn't "narrow" the type. Without the if block, line 63+ don't know that we're dealing with a result that is a question type. I'll see if I can change how we do that. I know we use the tinyinvariant library in places and I think that narrows properly.
| expect(result).toEqual(previewContent); | ||
| }); | ||
|
|
||
| it("does not mutate original question data", () => { |
2 participants
Summary:
This PR is part of a series building a typed, hook-based preview system for the Perseus editor. The new system replaces the untyped window.iframeDataStore + raw postMessage(string) communication with structured, validated message passing via usePreviewHost and usePreviewClient hooks. The new system is being built alongside the old one — no existing behaviour changes until the final PR in the series flips the switch.
Adds
sanitizeApiOptions()andsanitizePreviewData()which strip non-serializable values (function callbacks, React components) from preview data before it's sent viapostMessage. The structured clone algorithm used bypostMessagecan't handle functions, so they need to be removed before sending.sanitizeApiOptions()destructuresAPIOptionsto remove known non-serializable fields (onFocusChange,trackInteraction,imagePreloader,nativeKeypadProxy, placeholder components, etc.) and returns only the serializable remainder.sanitizePreviewData()wraps this for all preview content types — question, hint, article, and article-all. The article-all case currently sanitizes per-section since eachArticlePreviewDatacarries its ownapiOptions; this will be simplified when we restructure the preview data types in a later PR.Purely additive — no existing behaviour is changed.
Depends on: #3474
Issue: LEMS-3741
Test plan:
pnpm test