feat(core): forward portableTextBlocks and fieldWidgets for standard/sandboxed plugins#1484
Conversation
…sandboxed plugins Standard- and sandboxed-format plugins could declare adminPages and adminWidgets, but their declarative portableTextBlocks and fieldWidgets were dropped in adaptSandboxEntry, so only native-format plugins surfaced them. The admin editor already reads both from the manifest (slash-menu entries + Block Kit forms), so the data simply never arrived for non-native plugins. Forward them like the existing admin config: - adaptSandboxEntry copies descriptor.portableTextBlocks / fieldWidgets onto admin - PluginDescriptor and SandboxedPluginEntry carry the fields - the in-process and codegen plugin builders pass them through - the sandboxed-entry and marketplace manifest loops emit them The site-side render component (componentsEntry) remains native-only. Resolves the pre-existing TODO in the sandboxed-plugin manifest loop.
🦋 Changeset detectedLatest commit: 4571fac The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
|
Sorry about the label churn: I'm trying to fix the review bot |
automated event probe — dismissing
@ascorbic easy. Starting 2026 we are all slaves of our bots |
|
/review |
There was a problem hiding this comment.
Nice catch on the dropped portableTextBlocks/fieldWidgets — the in-process standard-format path (config-time descriptor → virtual-modules.ts → adaptSandboxEntry) is fully wired by this PR and that's the case the new tests exercise. Good.
However, the marketplace/R2 branches added in emdash-runtime.ts (reading bundle.manifest.admin?.portableTextBlocks / ?.fieldWidgets at lines ~943 and ~1884) are effectively dead — three independent layers strip those fields before they ever reach the runtime:
pluginAdminConfigSchemainplugins/manifest-schema.tsdeclaresfieldWidgetsbut notportableTextBlocksat all. With a plainz.object(...), Zod's default behaviour strips unknown keys, so even a hand-craftedmanifest.jsoncarryingadmin.portableTextBlockswould have it removed bysafeParse()inloadBundleFromR2andextractBundle.extractManifest()in bothcore/src/cli/commands/bundle-utils.tsandplugin-cli/src/bundle/utils.tsexplicitly writes only{ settingsSchema, pages, widgets }into the bundled manifest. NeitherportableTextBlocksnorfieldWidgetsis ever copied into amanifest.jsonwritten into a bundle.core/src/cli/commands/bundle.ts:336-345hard-errors (consola.error+process.exit(1)) when a plugin declaresportableTextBlocks, telling the author it requires native/trusted mode and cannot be bundled. Andplugin-cli/src/bundle/api.ts:217pluscore/src/cli/commands/bundle.ts:568still emit warnings to the same effect.
So for the actual marketplace path the PR description calls out ("the marketplace manifest cache carry them"), the new code can never read a non-undefined value. The new tests don't cover this path, and there's no integration test that round-trips a portableTextBlocks-carrying plugin through extractManifest → pluginManifestSchema → loadBundleFromR2.
To actually fix the marketplace branch, the PR would also need to (a) add portableTextBlocks to pluginAdminConfigSchema, (b) copy both fields in both extractManifest() implementations, (c) remove the bundler error in bundle.ts:336-345 and update the stale "will be ignored in sandboxed plugins" warnings in bundle.ts:568 and plugin-cli/src/bundle/api.ts:217, and (d) add a test that actually exercises an R2-loaded bundle. As-is, the in-process standard-format fix lands but the changeset and the resolved-TODO claim oversell the marketplace coverage.
One small nit on adminMode for sandboxed/marketplace plugins as well — see the inline note.
| w.size === "full" || w.size === "half" || w.size === "third" ? w.size : undefined, | ||
| })), | ||
| portableTextBlocks: bundle.manifest.admin?.portableTextBlocks, | ||
| fieldWidgets: bundle.manifest.admin?.fieldWidgets, |
There was a problem hiding this comment.
These two lines for marketplace bundles can't fire in practice. pluginManifestSchema in plugins/manifest-schema.ts doesn't include portableTextBlocks in pluginAdminConfigSchema (line 201–226), so Zod's default object behaviour silently strips it during pluginManifestSchema.safeParse(...) inside loadBundleFromR2. And extractManifest() in both core/src/cli/commands/bundle-utils.ts:161 and packages/plugin-cli/src/bundle/utils.ts:157 only copies { settingsSchema, pages, widgets } — portableTextBlocks and fieldWidgets are never written into the bundled manifest in the first place. On top of that, core/src/cli/commands/bundle.ts:336-345 hard-errors when a plugin declares portableTextBlocks, so the bundler refuses to produce such a bundle at all.
To make the marketplace branch real (rather than a no-op that just typechecks), the PR also needs to: extend pluginAdminConfigSchema to validate/preserve both fields, update both extractManifest() implementations to copy them, drop the hard-error in bundle.ts, and update the stale warning at plugin-cli/src/bundle/api.ts:217 and core/src/cli/commands/bundle.ts:568 ("will be ignored in sandboxed plugins"). Worth adding a round-trip test through extractManifest → pluginManifestSchema → loadBundleFromR2 to lock the behaviour in.
| w.size === "full" || w.size === "half" || w.size === "third" ? w.size : undefined, | ||
| })), | ||
| portableTextBlocks: bundle.manifest.admin?.portableTextBlocks, | ||
| fieldWidgets: bundle.manifest.admin?.fieldWidgets, |
There was a problem hiding this comment.
Same dead-branch issue as the earlier marketplace sync site at line 943-944: the manifest schema strips portableTextBlocks, neither extractManifest() writes the field, and the bundler currently refuses to emit it. This block will always observe undefined until the bundle/parse pipeline is updated.
| version: meta.version, | ||
| enabled, | ||
| sandboxed: true, | ||
| adminMode: hasAdminPages || hasWidgets ? "blocks" : "none", |
There was a problem hiding this comment.
Minor — adminMode for sandboxed and marketplace plugins is still computed only from hasAdminPages || hasWidgets. The configured-plugins branch above (line 2097-2102) has the same shape. With this PR, a plugin can now contribute portable-text blocks or field widgets and nothing else, in which case adminMode stays "none". That's currently harmless because getPluginBlocks in packages/admin/src/lib/pluginBlocks.ts reads portableTextBlocks straight from the manifest regardless of adminMode, and the sidebar only consults adminMode for admin-pages rendering. Worth a quick comment noting that adminMode === "none" doesn't imply "no admin contributions" anymore, so future readers don't gate on it.
|
|
||
| const result = adaptSandboxEntry(def, descriptor); | ||
|
|
||
| expect(result.admin.portableTextBlocks).toBeUndefined(); |
There was a problem hiding this comment.
The three new cases cover adaptSandboxEntry in isolation, which is fine, but the PR's headline claim ("the marketplace manifest cache carry them") isn't exercised anywhere. A test that drives a synthetic PluginBundle through pluginManifestSchema.safeParse → reconcileManifestAccess and asserts that admin.portableTextBlocks / admin.fieldWidgets survive would have caught the issue that the manifest schema strips portableTextBlocks. Worth adding before claiming the marketplace path works.
|
|
||
| Standard- and sandboxed-format plugins could already declare admin pages and dashboard widgets, but their declarative Portable Text block types and field widgets were dropped during adaptation — only native-format plugins surfaced them. Since the admin editor reads these from the manifest, the slash-menu entries and Block Kit forms never appeared for non-native plugins. | ||
|
|
||
| `adaptSandboxEntry` now forwards both, and the admin manifest is emitted for sandboxed and marketplace plugins too, so a plugin of any format can contribute Portable Text blocks and field widgets. The site-side render component (`componentsEntry`) still requires native format, which is unchanged. |
There was a problem hiding this comment.
The changeset says "the admin manifest is emitted for sandboxed and marketplace plugins too" — that's accurate for sandboxed-from-config (the virtual-modules.ts codegen path) but overclaims for marketplace bundles loaded from R2. As detailed in the review summary, the manifest schema and both extractManifest() implementations drop these fields, and the bundler refuses to emit them, so a marketplace-installed plugin still can't actually contribute a portable-text block via this fix. Consider scoping the changeset wording to standard/sandboxed-from-config until the bundle/parse layer is updated.
|
Review posted successfully. Summary: The PR correctly fixes the in-process standard-format plugin path (config-time descriptor → virtual modules →
Posted as a single batch review with the summary + 5 line-anchored comments on the relevant lines. |
… marketplace branches Addresses review on emdash-cms#1484: the marketplace/R2 reads of portableTextBlocks/ fieldWidgets were dead (manifest schema strips them, extractManifest doesn't write them, and the bundler deliberately refuses them). Remove those branches, scope the changeset accordingly, and note adminMode doesn't gate admin contributions. Full marketplace support is deferred to a follow-up.
|
Thanks for the thorough review — you're right that the marketplace/R2 path was dead (schema strips the fields, neither
The in-process and sandboxed-from-config paths (the ones the existing tests exercise) are unchanged and still forward both fields. Full marketplace support (extend |
|
/review |
|
The changes look clean and consistent. The author has:
The prior review's main concerns (dead marketplace branches, overclaiming changeset, missing adminMode comment) have all been addressed in commit 4571fac. The PR is now well-scoped. LGTM! |
There was a problem hiding this comment.
This is a focused, well-scoped fix. The problem is real: standard/sandboxed-format plugins could declare declarative portableTextBlocks and fieldWidgets, but adaptSandboxEntry only forwarded adminPages/adminWidgets, so the editor never saw them. The approach is consistent with how EmDash already treats these two formats — wrap the sandboxed default export and surface declarative admin contributions through the manifest — and the PR correctly leaves the native-only componentsEntry path untouched.
I checked the diff, traced where portableTextBlocks/fieldWidgets now flow (adaptSandboxEntry, generatePluginsModule, generateSandboxedPluginsModule, loadBypassedPlugins, and the manifest builder in EmDashRuntime), compared them against the existing adminPages/adminWidgets paths, verified the PluginDescriptor/SandboxedPluginEntry type updates, and reviewed the tests and changeset. The implementation is clean and the test covers the core adapter behavior plus the omission case. The marketplace/R2 bundle gap is explicitly out of scope and called out in the changeset, so I’m not treating that as a regression.
No issues found.
What does this PR do?
Standard- and sandboxed-format plugins can declare
portableTextBlocksandfieldWidgets, butadaptSandboxEntryonly copiedadminPages/adminWidgetsonto the resolvedadminconfig — so those two declarative fields were silently dropped for every non-native plugin. The admin editor already reads them from the manifest (slash-menu entries + Block Kit forms), so the data simply never arrived: a non-native plugin could never contribute a custom Portable Text block or field widget.This forwards both fields the same way
adminPages/adminWidgetsalready flow:plugins/adapt-sandbox-entry.ts— copydescriptor.portableTextBlocks/fieldWidgetsontoadmin.astro/integration/runtime.ts—PluginDescriptorcarries the two fields (typed via the existingPortableTextBlockConfig/FieldWidgetConfig).astro/integration/virtual-modules.ts— the in-processadaptSandboxEntry(...)call and the sandboxed-entry codegen pass them through.emdash-runtime.ts—SandboxedPluginEntryand the marketplace manifest cache carry them; the threeadaptSandboxEntrycall sites forward them; the sandboxed-entry and marketplace manifest loops emit them (this also resolves the pre-existingTODOin the sandboxed-plugin loop).The site-side render component (
componentsEntry) stays native-only — out of scope here.Closes #1483
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
```
$ pnpm --filter emdash typecheck # tsgo --noEmit — clean
$ pnpm --filter emdash exec vitest run tests/unit/plugins/adapt-sandbox-entry.test.ts
Test Files 1 passed (1)
Tests 28 passed (28) # +3 new: portable text blocks, field widgets, omitted-config
```
oxlint + prettier --check clean on the changed files. No admin UI strings were added (i18n N/A).