fix(core): scope the Svelte PDF context per <EmbedPDF> instance - #753
Open
claeyzre wants to merge 2 commits into
Open
fix(core): scope the Svelte PDF context per <EmbedPDF> instance#753claeyzre wants to merge 2 commits into
claeyzre wants to merge 2 commits into
Conversation
The Svelte adapter kept its context in one module-level $state object that every consumer read, so two <EmbedPDF> instances on a page overwrote each other's registry, coreState and activeDocumentId, and unmounting either one reset both to their empty state. Each instance now creates its own context and publishes it with setContext, which is how the React, Preact and Vue adapters already scope theirs. usePlugin resolves through useRegistry() instead of importing the module object, matching the other adapters. The context key uses Symbol.for so two accidentally bundled copies of @embedpdf/core still agree on it. Behaviour changes: - Components with no <EmbedPDF> ancestor previously reached the global object and worked by accident. They now get an inert, frozen fallback and a console warning on every resolution. The warning is deliberately not once-only: a module-level flag would be shared across SSR requests, silencing it for every request after the first. - useRegistry, useCoreState and usePlugin must be called during component initialization, as getContext requires. useCapability already had this constraint through its internal $effect. - pdfContext stays exported so existing imports resolve, but it is deprecated, frozen, and no longer written to; writes now throw instead of leaking into other consumers. Adds a vitest suite (the package had no test setup) covering instance isolation, snippet/descendant resolution, sibling-unmount survival, and the orphan fallback path. All six tests fail against the previous implementation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JmxLnPJcJaA8s885gY4rgZ
Mounts two independent <EmbedPDF> instances side by side, each with its own registry and document, plus a toggle that unmounts the second one. Each panel prints the context it resolved, so the isolation is visible without opening the devtools. Against the previous module-level context both panels report the same activeDocumentId and render nothing, and the console shows "Cannot register viewport for <id>: document state not found". A third probe sits outside every <EmbedPDF> to show the one case the fix changes: it resolves the inert fallback and warns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JmxLnPJcJaA8s885gY4rgZ
|
Someone is attempting to deploy a commit to the CloudPDF Team on Vercel. A member of the Team first needs to authorize it. |
claeyzre
marked this pull request as ready for review
September 1, 2026 07:45
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.
Fixes #752.
What
The Svelte adapter keeps its context in one module-level
$stateobject that every consumer reads, so two<EmbedPDF>instances on a page overwrite each other'sregistry,coreStateandactiveDocumentId, and unmounting either one resets both.Each instance now creates its own context and publishes it with
setContext, the way the React, Preact and Vue adapters already scope theirs.usePlugingoes throughuseRegistry()instead of importing the module object directly, also matching the others.Three files in
packages/core/src/svelte, a new test suite, and a changeset:hooks/use-registry.svelte.ts—createPdfContext()mints a fresh reactive context;setPdfContext/useRegistryusesetContext/getContext. The key isSymbol.for('@embedpdf/core:pdf-context'), so two accidentally bundled copies of the package still agree on it.hooks/use-plugin.svelte.ts—useRegistry()instead of the module importcomponents/EmbedPDF.svelte— two lines to create and publish the context; everything below is untouched because the local binding keeps the nameTests
packages/corehad no test setup, so this adds one: vitest + jsdom +@sveltejs/vite-plugin-svelte, wired aspnpm --filter @embedpdf/core test. Six tests intests/svelte/context.test.tscover:<EmbedPDF>ancestor get the inert fallback and a warningAll six fail against the current
v2implementation — verified by swapping the oldsrc/svelteback in and re-running.Before / after
The second commit adds
examples/svelte-tailwind/src/routes/dual, which mounts two instances side by side and prints the context each one resolves.Against
v2as it stands, both panels report the sameactiveDocumentId, both render nothing, and the console explains why:One panel's document id is being used against the other panel's registry.
With the fix, the panels report distinct ids, both render, and the console is clean. Unmounting one leaves the other fully intact.
The behaviour this changes
Worth deciding on explicitly before merging.
Components mounted outside an
<EmbedPDF>stop resolving a registry. Today a toolbar sitting next to<EmbedPDF>rather than inside it can calluseZoomCapability()and it works, because the object is global. After this change it gets a frozen fallback that nothing writes to, so it seesregistry: nullandisLoading: true, and this warning:The warning fires on every resolution rather than once. That's deliberate: a module-level "warn once" flag would be shared across SSR requests — the same cross-instance leak this PR removes — and would silence the warning for every request after the first on a long-running server.
React degrades the same way (default context, never ready); Vue throws. Happy to switch to throwing if you'd rather match Vue — say the word and I'll push it.
useRegistry(),useCoreState()andusePlugin()must now be called during component initialization, asgetContextrequires. Narrower than it sounds:useCapabilitycalls$effectinternally, so it already throwseffect_orphanoutside an init context. Only the three direct hooks change. Every plugin hook in the repo calls them at the top of its own function body, so none of them move.pdfContextis still exported so existing imports resolve, but it is deprecated, frozen, and no longer written to. A consumer still writing to it gets aTypeErrorinstead of silently feeding state to unrelated components.Notes on the implementation
setContext/getContextwith a symbol key rather thancreateContext, which needs Svelte 5.40 while@embedpdf/coredeclares"svelte": ">=5 <6". Raising that floor felt like your call.const { registry } = …read inusePluginis left as it was. This PR scopes the context; it doesn't change when plugins resolve.plugin-viewportsetsviewport-element,plugin-uiandplugin-annotationkeep registries), so this introduces no new mechanism.patchon@embedpdf/core. Given the out-of-tree behaviour change you may preferminor— your call, happy to bump it.Verified
pnpm --filter @embedpdf/core test— 6/6 pass; 6/6 fail with the old implementation swapped back inpnpm --filter @embedpdf/core build— all five modes (base, react, preact, vue, svelte) exit 0svelte-checkonexamples/svelte-tailwindreports nothing from the new route and nothing touching this changeprettier --checkclean on every file touchedeslint src/sveltereports the same 16 findings before and after — no new ones (the$state/console"not defined" errors are the existing rune/browser-globals gap in the config, which already hitssrc/shared/components/embed-pdf.tsx)The second commit is optional
docs(example-svelte)is separate on purpose. Drop it if you'd rather not carry a demo route — the fix and its tests stand on their own.Related
#520 touches the same branch in
EmbedPDF.svelteand interacts with this. The branch swap onpluginsReadyis what makes the bug visible, but it is also the only thing that lets a SvelteusePluginconsumer pick up a registry at all, because it destructures once and never re-reads. Removing the remount without making that read reactive would leave consumers that initialize before the registry lands stuck onisLoading: true. Flagging it so the two aren't fixed independently.