✨ Added plugin card system for custom editor cards#1989
Closed
danielperez9430 wants to merge 4 commits into
Closed
✨ Added plugin card system for custom editor cards#1989danielperez9430 wants to merge 4 commits into
danielperez9430 wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces a Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
packages/kg-default-nodes/src/nodes/plugin-card/PluginCardNode.ts (1)
9-9: ⚡ Quick winDuplicated default values may diverge.
The
payloaddefault'{}'is defined both inpluginCardProperties(line 9) and again indefaults(line 50). If one changes without the other, import behavior could differ from node creation defaults.Consider extracting defaults from
pluginCardPropertiesto ensure consistency:♻️ Suggested refactor to derive defaults from properties
const data: Record<string, unknown> = {}; const propNames = ['html', 'pluginName', 'cardName', 'payload', 'css', 'template', 'preprocess']; - const defaults: Record<string, unknown> = {html: '', pluginName: '', cardName: '', payload: '{}', css: '', template: '', preprocess: ''}; + const defaults = Object.fromEntries( + pluginCardProperties.map(p => [p.name, p.default]) + ); propNames.forEach((name) => {Also applies to: 50-50
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/kg-default-nodes/src/nodes/plugin-card/PluginCardNode.ts` at line 9, The `payload` default value `'{}'` is duplicated in both `pluginCardProperties` (line 9) and the `defaults` object (line 50), creating a maintenance risk where changes in one location could be forgotten in the other. Extract the default values from `pluginCardProperties` by deriving the defaults object from that property definition instead of maintaining separate hardcoded defaults. Specifically, remove the duplicate `{name: 'payload', default: '{}'}` entry from the `defaults` object at line 50 and instead programmatically derive the defaults by mapping over the `pluginCardProperties` to extract each property's default value, ensuring a single source of truth for all default values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/kg-default-nodes/src/nodes/plugin-card/plugin-card-css.ts`:
- Around line 52-59: The scopeInnerRules function (lines 141–146) is currently a
no-op and does not actually scope selectors within `@media` blocks, allowing CSS
rules to leak globally instead of being namespaced. Either implement the
scopeInnerRules function to properly parse and prefix selectors inside `@media`
rules with the namespace, or add clear documentation explaining that `@media`
block scoping is not supported and selectors within them will not be isolated.
The current comment claiming selectors already match outer scoped rules is
misleading and should be corrected or removed if you choose the documentation
approach.
In `@packages/kg-default-nodes/src/nodes/plugin-card/plugin-card-preprocess.ts`:
- Around line 26-27: The String, Number, Boolean, and Date constructors are
passed directly to the sandbox without protection, allowing malicious preprocess
code to modify their prototypes and cause prototype pollution. Wrap these
constructors with frozen or protected versions similar to how the other globals
are already wrapped in the sandbox initialization to prevent prototype
modifications. If the preprocess code needs to create Date instances, provide a
factory function wrapper instead of exposing the direct Date constructor.
In `@packages/kg-default-nodes/src/nodes/plugin-card/plugin-card-renderer.ts`:
- Around line 41-57: The addDataAttributes function creates a data-ghost-payload
attribute using single-quote delimiters, but the payloadStr variable only
escapes double quotes via JSON.stringify. If the payload contains single quotes
(like {"name": "it's"}), the attribute syntax breaks. Fix this by either: (1)
escaping single quotes in payloadStr before constructing the data-ghost-payload
attribute string, or (2) switching the data-ghost-payload attribute to use
double-quote delimiters instead of single quotes (which already has proper
escaping in place from the JSON.stringify call). Choose the approach that best
fits your project's conventions.
In `@packages/kg-default-nodes/src/nodes/plugin-card/plugin-card-template.ts`:
- Around line 45-51: The escapeHtml function is missing the escape sequence for
single quotes, which can cause rendering issues or allow attribute injection
when single-quoted attributes contain interpolated values. Add a replace call to
the escapeHtml function chain to escape single quotes (') to &`#39`; using the
same pattern as the existing replace calls for other special characters like &,
<, >, and ".
In `@packages/koenig-lexical/src/context/PluginCardContext.jsx`:
- Around line 18-23: The catch block in the plugin cards fetch flow sets
_pluginCardsLoaded to true and returns an empty array on failure, but fails to
reset _pluginCardsPromise, causing subsequent calls to return the cached empty
result instead of retrying. In the catch block where _pluginCardsLoaded is set
to true, also reset _pluginCardsPromise to null so that the next call to
getPluginCards will create a new promise and attempt to fetch again rather than
returning the permanently failed result.
In `@packages/koenig-lexical/src/nodes/PluginCardNodeComponent.jsx`:
- Around line 73-89: The conditional checks for updating node.css and
node.template properties use truthy checks instead of nullish checks, which
prevents clearing stale values when the found properties are falsy but defined
(like empty strings). In the PluginCardNodeComponent.jsx file where the
editor.update block syncs template, CSS and preprocess from the plugin loader,
change the conditions for node.css and node.template updates from truthy checks
(if (found.css) and if (found.template)) to nullish checks using the same
pattern as the existing node.preprocess check (if (found.css !== undefined) and
if (found.template !== undefined)). This ensures that all property updates,
including falsy values, will overwrite stale node metadata.
- Around line 65-96: The useEffect in PluginCardNodeComponent is making
independent fetch requests to '/ghost/api/admin/plugins/cards/' for each card
instance, which bypasses the PluginCardContext's single-flight cache and causes
duplicate requests when multiple plugin cards are rendered. Replace the
individual fetch call with consumption of PluginCardContext to retrieve the
shared cached plugin data instead. This will eliminate the redundant network
requests by allowing all card instances to use a single fetched set of plugin
definitions from the context.
- Around line 33-45: The issue is that all card instances of the same plugin
share a single stylesheet identified by styleId, but the cleanup function
unconditionally removes that shared style element whenever any instance
unmounts, causing other mounted cards of the same plugin to lose their styles.
Implement reference counting for the shared style element: create a mechanism to
track how many component instances are currently using each stylesheet (keyed by
styleId), increment the count when the component mounts, and only remove the
style element from the DOM in the cleanup function when the reference count
reaches zero after decrementing. This ensures the shared stylesheet persists as
long as at least one card instance for that plugin remains mounted.
In `@packages/koenig-lexical/src/plugins/PluginCardPlugin.jsx`:
- Around line 21-26: Remove the async keyword from the handler function that
accepts the dataset parameter in the PluginCardPlugin.jsx file. The handler must
return a boolean synchronously to properly control command propagation in
Lexical's command system; currently it returns a Promise which Lexical does not
use for determining if the command was handled. Change the handler to
synchronously execute the dispatchCommand call and return its result directly.
In `@packages/koenig-lexical/src/plugins/PlusCardMenuPlugin.jsx`:
- Line 20: The usePluginCards() hook in PlusCardMenuPlugin cannot access the
PluginCardProvider context because the provider is only wrapping
PluginCardPlugin in AllDefaultPlugins.jsx, not CardMenuPlugin which renders
PlusCardMenuPlugin. To fix this, restructure the component tree in
AllDefaultPlugins.jsx so that PluginCardProvider wraps both CardMenuPlugin (line
39) and PluginCardPlugin (line 61) together, ensuring the provider is a common
ancestor to all components that need to call the usePluginCards hook.
Alternatively, move PluginCardProvider higher in the component hierarchy to be
an ancestor of both components.
In `@packages/koenig-lexical/src/utils/buildCardMenu.js`:
- Around line 125-131: The PluginCardIcon function directly injects
pluginCard.icon into dangerouslySetInnerHTML without sanitization, allowing
malicious plugins to execute scripts or event handlers in admin context.
Sanitize the pluginCard.icon value before using it in the
dangerouslySetInnerHTML attribute. Apply a sanitization function or library
(such as DOMPurify) to strip out potentially dangerous content like script tags
and event handlers from the SVG string before rendering it.
- Around line 121-123: The buildCardMenu function assumes plugin cards have a
valid shape with required properties, but does not validate them before use. Add
guard clauses to validate the pluginCard structure before accessing its
properties. Specifically, before line 121 where pluginCard.label is accessed
with toLowerCase(), verify that the label property exists and is a string.
Similarly, before line 145 where fields.reduce() is called, verify that the
fields property exists and is an array. These guards should either skip
malformed cards or provide safe defaults, preventing one invalid plugin card
from breaking the entire card menu build process.
- Around line 13-16: Remove the debug logging statements from the buildCardMenu
function that execute during frequent menu rebuilds. Delete the console.log
statement (lines 13-16) that logs plugin cards from the config, and also remove
the debug logging statement at lines 74-75. These statements add unnecessary
console noise and create performance overhead during common operations like
slash command typing.
---
Nitpick comments:
In `@packages/kg-default-nodes/src/nodes/plugin-card/PluginCardNode.ts`:
- Line 9: The `payload` default value `'{}'` is duplicated in both
`pluginCardProperties` (line 9) and the `defaults` object (line 50), creating a
maintenance risk where changes in one location could be forgotten in the other.
Extract the default values from `pluginCardProperties` by deriving the defaults
object from that property definition instead of maintaining separate hardcoded
defaults. Specifically, remove the duplicate `{name: 'payload', default: '{}'}`
entry from the `defaults` object at line 50 and instead programmatically derive
the defaults by mapping over the `pluginCardProperties` to extract each
property's default value, ensuring a single source of truth for all default
values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7201a894-5daa-4f3e-b611-caf03cf5f8c4
📒 Files selected for processing (19)
packages/kg-default-nodes/src/kg-default-nodes.tspackages/kg-default-nodes/src/nodes/plugin-card/PluginCardNode.tspackages/kg-default-nodes/src/nodes/plugin-card/index.tspackages/kg-default-nodes/src/nodes/plugin-card/plugin-card-css.tspackages/kg-default-nodes/src/nodes/plugin-card/plugin-card-parser.tspackages/kg-default-nodes/src/nodes/plugin-card/plugin-card-preprocess.tspackages/kg-default-nodes/src/nodes/plugin-card/plugin-card-renderer.tspackages/kg-default-nodes/src/nodes/plugin-card/plugin-card-template.tspackages/koenig-lexical/src/commands/pluginCardCommands.jspackages/koenig-lexical/src/context/PluginCardContext.jsxpackages/koenig-lexical/src/index.jspackages/koenig-lexical/src/nodes/DefaultNodes.jspackages/koenig-lexical/src/nodes/PluginCardNode.jsxpackages/koenig-lexical/src/nodes/PluginCardNodeComponent.jsxpackages/koenig-lexical/src/plugins/AllDefaultPlugins.jsxpackages/koenig-lexical/src/plugins/PluginCardPlugin.jsxpackages/koenig-lexical/src/plugins/PlusCardMenuPlugin.jsxpackages/koenig-lexical/src/plugins/SlashCardMenuPlugin.jsxpackages/koenig-lexical/src/utils/buildCardMenu.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/kg-default-nodes/src/nodes/plugin-card/plugin-card-renderer.ts`:
- Around line 70-71: The deep copy operation using
JSON.parse(JSON.stringify(rawPayload)) on line 71 can throw an error when
rawPayload contains non-JSON-serializable objects such as circular references.
Wrap this operation in a try-catch block to guard against serialization failures
before preprocess runs. In the catch block, handle the error appropriately by
either logging the error and using the original rawPayload, or throwing a
descriptive error message to prevent silent failures during export.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0bfeb8fa-db6f-45a3-90fa-c32b63d93256
📒 Files selected for processing (1)
packages/kg-default-nodes/src/nodes/plugin-card/plugin-card-renderer.ts
danielperez9430
force-pushed
the
main
branch
2 times, most recently
from
27d7b70 to
7e9e0e2
Compare
refs TryGhost#1980 Ghost admins can now install custom editor cards via ZIP upload. Cards define their own Handlebars template, CSS, form fields, and optional preprocess hook for data transformation. The system includes: - PluginCardNode with generic template rendering (renderTemplate engine) - Sandboxed preprocess hook (createPreprocessor with frozen constructors) - Dynamic card menu population via PluginCardProvider context - CSS namespace isolation (scopeCss) and editor preflight reset - ZIP Slip and path traversal protections in plugin loader and extraction - HTML attribute escaping and DOMPurify sanitization for plugin icons - Reference counting for shared plugin stylesheets
Collaborator
|
Closing - see TryGhost/Ghost#28642 for details. |
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
2 participants
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.
Adds a plugin card system that lets Ghost admins install custom editor cards via ZIP upload. Cards define their own template (Handlebars syntax), CSS, and form fields — enabling review cards, info boxes, pricing tables, and more without touching Koenig's source code.
This PR must be merged and published to npm before the companion Ghost PR, because Ghost's
plugin-renderer.jsimportsrenderTemplateandcreatePreprocessorfrom@tryghost/kg-default-nodes.Companion PR: TryGhost/Ghost#28642
New Packages/Files
kg-default-nodes— Base node & renderingPluginCardNode.ts— Lexical node for plugin cards withpluginName,cardName,payload,template,css, andpreprocesspropertiesplugin-card-template.ts— Lightweight Handlebars-compatible template engine ({{var}},{{#if}},{{#each}},{{#unless}})plugin-card-renderer.ts— Client-sideexportDOMrenderer using the template engineplugin-card-parser.ts—importDOMparser that extractsdata-ghost-*attributes for re-importplugin-card-css.ts—scopeCss()utility that namespaces selectors to prevent style collisionsplugin-card-preprocess.ts— SandboxedcreatePreprocessor()that runs plugin-defined data transformations with frozen wrapper objects to prevent prototype pollutionkoenig-lexical— Editor integrationPluginCardNode.jsx— Editor node subclass with icon, indicator, andKoenigCardWrapperPluginCardNodeComponent.jsx— React component with:cardDef.fields)dangerouslySetInnerHTMLnot-kg-prosewrapper for Koenig typography isolationPluginCardPlugin.jsx— Lexical plugin that registers theINSERT_PLUGIN_CARD_COMMANDPluginCardContext.jsx— React context that fetches plugin cards from the API and populates the slash/plus menupluginCardCommands.js— Lexical command definitionModified files
kg-default-nodes.ts— Re-exportsrenderTemplate,scopeCss,createPreprocessorDefaultNodes.js— RegistersPluginCardNodeAllDefaultPlugins.jsx— RegistersPluginCardPluginbuildCardMenu.js— Builds menu items from plugin card configSlashCardMenuPlugin.jsx/PlusCardMenuPlugin.jsx— Pass plugin cards to menu builderPluginCardNode.ts— Property list updated withtemplateandpreprocessFeatures
template.htmlusing Handlebars syntax ({{var}},{{#if}},{{#each}},{{#unless}},{{else}}). Renders identically in editor and published page.plugin.jsonfield definitions (text,number,textarea)scopeCss()adds.plugin-{name}prefix to all selectors. Editor view mode applies a preflight reset to neutralise Tailwind overrides.importDOMextractsdata-ghost-plugin,data-card-name,data-ghost-payloadfrom HTMLpreprocess.jsthat transforms raw form data before rendering (e.g., parsing"Label=9"into structured rating objects). Runs in a sandboxed environment.Testing
/or click+→ plugin card appears in menuScreenshots