Migrate kg-lexical-html-renderer to TypeScript#1791
Conversation
WalkthroughThis PR converts the package to an ESM-first source layout (sources moved under src/), adds tsconfig variants for CJS and tests, and updates package.json to produce dual ESM/CJS builds with an exports map. It migrates many modules and tests from CommonJS to ESM with explicit .js specifiers, introduces new src entrypoints and transformer typings, and removes several lib .d.ts ambient declarations. LexicalHTMLRenderer was changed to lazy async DOM initialization and updated typings; numerous renderer/node return types and helper utilities (getFirstHtmlElement, visibility guards) were introduced or adjusted. ESLint config was migrated to flat-config. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/kg-lexical-html-renderer/test/cards.test.ts (1)
9-10: Prefer explicit test-local shapes overRecord<string, unknown>.Lines 9–10 use overly broad types that prevent TypeScript from validating nested property access. The test code actively depends on concrete nested fields (
root.children,imageOptimization.contentImageSizes,createDocument,target), and narrowing these to explicit interfaces will restore type safety instead of falling back tounknown.♻️ Proposed typing refinement
- let lexicalState: Record<string, unknown>; - let options: Record<string, unknown>; + type LexicalState = { + root: { + children: Array<Record<string, unknown>>; + direction: string | null; + format: string; + indent: number; + type: 'root'; + version: number; + }; + }; + + type RenderOptions = { + imageOptimization: { + contentImageSizes: Record<string, {width: number}>; + }; + createDocument: () => unknown; + target?: 'email'; + }; + + let lexicalState: LexicalState; + let options: RenderOptions;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/test/cards.test.ts` around lines 9 - 10, The test declares overly broad types for lexicalState and options which prevents TS from validating nested accesses; replace Record<string, unknown> with small explicit interfaces matching the test's usage (include properties accessed like root: { children: unknown[] } or a proper Node[] shape for lexicalState, and for options include imageOptimization: { contentImageSizes: number[] }, createDocument: Function type matching usage, and target: string|Element as used), update the variable declarations for lexicalState and options to those interfaces, and adjust any helper usage (e.g., createDocument, imageOptimization.contentImageSizes, root.children, target) so TypeScript can type-check nested property access.packages/kg-lexical-html-renderer/test/utils/should-render.ts (1)
4-4: Create a freshJSDOMper test case.Line 4 shares a single DOM across every
shouldRenderinvocation. If any renderer path mutates the document or window state, later tests can become order-dependent. Construct the DOM inside the returned test function instead.♻️ Possible cleanup
-const dom = new JSDOM(); - function shouldRender({input, output, options = {}}: ShouldRenderParams) { return async function () { + const dom = new JSDOM(); const defaultOnError = (err: Error) => { throw err; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/test/utils/should-render.ts` at line 4, The shared JSDOM instance (const dom = new JSDOM()) causes state to leak between invocations of shouldRender; change the code so a new JSDOM() is constructed inside the returned test function instead of at module scope. Locate the shouldRender helper (the function that returns the test callback) and move the DOM creation into that returned callback so each test call gets its own dom/window/document, ensuring any mutations are isolated per invocation.packages/kg-lexical-html-renderer/package.json (1)
19-21: Extract the shared build chain once.Lines 19-21 repeat the same build pipeline three times. Pulling that into a dedicated script will keep
build,prepare, andpretestfrom drifting the next time the output layout changes.♻️ Possible cleanup
"scripts": { "dev": "tsc --watch --preserveWatchOutput --sourceMap", - "build": "tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", - "prepare": "tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", - "pretest": "tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json && tsc -p tsconfig.test.json", + "build:dist": "tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", + "build": "yarn build:dist", + "prepare": "yarn build:dist", + "pretest": "yarn build:dist && tsc -p tsconfig.test.json",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/package.json` around lines 19 - 21, The build/prepare/pretest scripts in package.json duplicate the same pipeline; extract the common sequence into a single npm script (e.g., "build:core" or similar) and have "build", "prepare", and "pretest" call that script instead of repeating the commands. Update the package.json script entries "build", "prepare", and "pretest" to invoke the new shared script, ensuring the original behavior (running tsc, tsc -p tsconfig.cjs.json, and writing build/esm/package.json) is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/kg-lexical-html-renderer/package.json`:
- Line 23: The test script "test:unit" hardcodes node_modules/.bin/mocha which
breaks in hoisted workspaces; update the package.json "test:unit" script to
invoke mocha via the package-manager PATH (i.e., call mocha directly) and use
tsx only to register the TypeScript loader (e.g., run tsx to launch mocha rather
than referencing node_modules/.bin/mocha), so replace the
node_modules/.bin/mocha segment with just mocha in the "test:unit" script.
---
Nitpick comments:
In `@packages/kg-lexical-html-renderer/package.json`:
- Around line 19-21: The build/prepare/pretest scripts in package.json duplicate
the same pipeline; extract the common sequence into a single npm script (e.g.,
"build:core" or similar) and have "build", "prepare", and "pretest" call that
script instead of repeating the commands. Update the package.json script entries
"build", "prepare", and "pretest" to invoke the new shared script, ensuring the
original behavior (running tsc, tsc -p tsconfig.cjs.json, and writing
build/esm/package.json) is preserved.
In `@packages/kg-lexical-html-renderer/test/cards.test.ts`:
- Around line 9-10: The test declares overly broad types for lexicalState and
options which prevents TS from validating nested accesses; replace
Record<string, unknown> with small explicit interfaces matching the test's usage
(include properties accessed like root: { children: unknown[] } or a proper
Node[] shape for lexicalState, and for options include imageOptimization: {
contentImageSizes: number[] }, createDocument: Function type matching usage, and
target: string|Element as used), update the variable declarations for
lexicalState and options to those interfaces, and adjust any helper usage (e.g.,
createDocument, imageOptimization.contentImageSizes, root.children, target) so
TypeScript can type-check nested property access.
In `@packages/kg-lexical-html-renderer/test/utils/should-render.ts`:
- Line 4: The shared JSDOM instance (const dom = new JSDOM()) causes state to
leak between invocations of shouldRender; change the code so a new JSDOM() is
constructed inside the returned test function instead of at module scope. Locate
the shouldRender helper (the function that returns the test callback) and move
the DOM creation into that returned callback so each test call gets its own
dom/window/document, ensuring any mutations are isolated per invocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f8589c9-47c3-48a9-baed-e64ec9f50998
📒 Files selected for processing (33)
packages/kg-lexical-html-renderer/eslint.config.mjspackages/kg-lexical-html-renderer/lib/index.tspackages/kg-lexical-html-renderer/lib/transformers/index.tspackages/kg-lexical-html-renderer/package.jsonpackages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.tspackages/kg-lexical-html-renderer/src/convert-to-html-string.tspackages/kg-lexical-html-renderer/src/get-dynamic-data-nodes.tspackages/kg-lexical-html-renderer/src/index.tspackages/kg-lexical-html-renderer/src/kg-default-nodes.d.tspackages/kg-lexical-html-renderer/src/kg-utils.d.tspackages/kg-lexical-html-renderer/src/transformers/element/aside.tspackages/kg-lexical-html-renderer/src/transformers/element/blockquote.tspackages/kg-lexical-html-renderer/src/transformers/element/heading.tspackages/kg-lexical-html-renderer/src/transformers/element/list.tspackages/kg-lexical-html-renderer/src/transformers/element/paragraph.tspackages/kg-lexical-html-renderer/src/transformers/index.tspackages/kg-lexical-html-renderer/src/utils/TextContent.tspackages/kg-lexical-html-renderer/src/utils/generate-id.tspackages/kg-lexical-html-renderer/test/cards.test.tspackages/kg-lexical-html-renderer/test/headings.test.tspackages/kg-lexical-html-renderer/test/links.test.tspackages/kg-lexical-html-renderer/test/lists.test.tspackages/kg-lexical-html-renderer/test/quotes.test.tspackages/kg-lexical-html-renderer/test/render.test.tspackages/kg-lexical-html-renderer/test/text-formats.test.tspackages/kg-lexical-html-renderer/test/utils/assertions.tspackages/kg-lexical-html-renderer/test/utils/index.tspackages/kg-lexical-html-renderer/test/utils/overrides.tspackages/kg-lexical-html-renderer/test/utils/should-render.jspackages/kg-lexical-html-renderer/test/utils/should-render.tspackages/kg-lexical-html-renderer/tsconfig.cjs.jsonpackages/kg-lexical-html-renderer/tsconfig.jsonpackages/kg-lexical-html-renderer/tsconfig.test.json
💤 Files with no reviewable changes (3)
- packages/kg-lexical-html-renderer/lib/index.ts
- packages/kg-lexical-html-renderer/test/utils/should-render.js
- packages/kg-lexical-html-renderer/lib/transformers/index.ts
89df277 to
f75ef91
Compare
e1a2f23 to
939ccf2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/kg-lexical-html-renderer/eslint.config.mjs (1)
10-21: Turn offno-undeffor TypeScript files.
eslint.configs.recommendedenablesno-undef, and both ESLint and typescript-eslint document that this rule should generally be disabled on TypeScript because it doesn't understand TS/global types and is handled by the compiler. Add'no-undef': 'off'in this TS block before layering package-specific rules. (eslint.org)♻️ Proposed fix
rules: { ...ghostPlugin.configs.ts.rules, + 'no-undef': 'off', '@typescript-eslint/no-explicit-any': 'error' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/eslint.config.mjs` around lines 10 - 21, The TypeScript ESLint config is enabling the core no-undef rule via eslint.configs.recommended; update the rules object in the TS block (the object that currently spreads ghostPlugin.configs.ts.rules and sets '@typescript-eslint/no-explicit-any') to explicitly disable the core rule by adding 'no-undef': 'off' before layering package-specific rules (i.e., place it alongside the other entries in the rules object, prior to or immediately before ...ghostPlugin.configs.ts.rules) so TypeScript/ts-eslint handles undefined globals instead of core ESLint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/kg-lexical-html-renderer/package.json`:
- Around line 10-16: The new "exports" field in package.json restricts package
subpaths and will break consumers that rely on deep imports (e.g.,
build/transformers/* or build/utils/* such as
`@tryghost/kg-lexical-html-renderer/build/transformers/element/aside.js`); either
remove or relax the "exports" map or explicitly add the needed subpath entries
under "exports" to re-export build/transformers/* and build/utils/*, or
otherwise prepare this as a major-version breaking change; locate the
package.json "exports" entry and update it to include explicit subpath keys for
the existing deep import folders (or revert the addition) so existing consumers
continue to work.
---
Nitpick comments:
In `@packages/kg-lexical-html-renderer/eslint.config.mjs`:
- Around line 10-21: The TypeScript ESLint config is enabling the core no-undef
rule via eslint.configs.recommended; update the rules object in the TS block
(the object that currently spreads ghostPlugin.configs.ts.rules and sets
'@typescript-eslint/no-explicit-any') to explicitly disable the core rule by
adding 'no-undef': 'off' before layering package-specific rules (i.e., place it
alongside the other entries in the rules object, prior to or immediately before
...ghostPlugin.configs.ts.rules) so TypeScript/ts-eslint handles undefined
globals instead of core ESLint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5649d4d-854b-4f59-93cc-3bacb509e2f4
📒 Files selected for processing (30)
packages/kg-lexical-html-renderer/eslint.config.mjspackages/kg-lexical-html-renderer/lib/index.tspackages/kg-lexical-html-renderer/lib/transformers/index.tspackages/kg-lexical-html-renderer/package.jsonpackages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.tspackages/kg-lexical-html-renderer/src/convert-to-html-string.tspackages/kg-lexical-html-renderer/src/get-dynamic-data-nodes.tspackages/kg-lexical-html-renderer/src/index.tspackages/kg-lexical-html-renderer/src/kg-default-nodes.d.tspackages/kg-lexical-html-renderer/src/transformers/element/aside.tspackages/kg-lexical-html-renderer/src/transformers/element/blockquote.tspackages/kg-lexical-html-renderer/src/transformers/element/heading.tspackages/kg-lexical-html-renderer/src/transformers/element/list.tspackages/kg-lexical-html-renderer/src/transformers/element/paragraph.tspackages/kg-lexical-html-renderer/src/transformers/index.tspackages/kg-lexical-html-renderer/src/utils/TextContent.tspackages/kg-lexical-html-renderer/src/utils/generate-id.tspackages/kg-lexical-html-renderer/test/cards.test.tspackages/kg-lexical-html-renderer/test/headings.test.tspackages/kg-lexical-html-renderer/test/links.test.tspackages/kg-lexical-html-renderer/test/lists.test.tspackages/kg-lexical-html-renderer/test/quotes.test.tspackages/kg-lexical-html-renderer/test/render.test.tspackages/kg-lexical-html-renderer/test/text-formats.test.tspackages/kg-lexical-html-renderer/test/utils/index.tspackages/kg-lexical-html-renderer/test/utils/overrides.tspackages/kg-lexical-html-renderer/test/utils/should-render.tspackages/kg-lexical-html-renderer/tsconfig.cjs.jsonpackages/kg-lexical-html-renderer/tsconfig.jsonpackages/kg-lexical-html-renderer/tsconfig.test.json
💤 Files with no reviewable changes (2)
- packages/kg-lexical-html-renderer/lib/transformers/index.ts
- packages/kg-lexical-html-renderer/lib/index.ts
✅ Files skipped from review due to trivial changes (16)
- packages/kg-lexical-html-renderer/test/quotes.test.ts
- packages/kg-lexical-html-renderer/src/utils/generate-id.ts
- packages/kg-lexical-html-renderer/test/headings.test.ts
- packages/kg-lexical-html-renderer/test/links.test.ts
- packages/kg-lexical-html-renderer/src/get-dynamic-data-nodes.ts
- packages/kg-lexical-html-renderer/tsconfig.cjs.json
- packages/kg-lexical-html-renderer/src/utils/TextContent.ts
- packages/kg-lexical-html-renderer/test/lists.test.ts
- packages/kg-lexical-html-renderer/src/kg-default-nodes.d.ts
- packages/kg-lexical-html-renderer/tsconfig.test.json
- packages/kg-lexical-html-renderer/test/cards.test.ts
- packages/kg-lexical-html-renderer/src/convert-to-html-string.ts
- packages/kg-lexical-html-renderer/test/text-formats.test.ts
- packages/kg-lexical-html-renderer/src/index.ts
- packages/kg-lexical-html-renderer/test/render.test.ts
- packages/kg-lexical-html-renderer/src/transformers/index.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/kg-lexical-html-renderer/test/utils/index.ts
- packages/kg-lexical-html-renderer/src/transformers/element/aside.ts
- packages/kg-lexical-html-renderer/src/transformers/element/list.ts
- packages/kg-lexical-html-renderer/tsconfig.json
- packages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.ts
- packages/kg-lexical-html-renderer/test/utils/should-render.ts
- packages/kg-lexical-html-renderer/src/transformers/element/heading.ts
- packages/kg-lexical-html-renderer/src/transformers/element/blockquote.ts
- packages/kg-lexical-html-renderer/src/transformers/element/paragraph.ts
- packages/kg-lexical-html-renderer/test/utils/overrides.ts
939ccf2 to
29d24b4
Compare
29d24b4 to
6e95756
Compare
37b0fef to
076d830
Compare
076d830 to
3d7be43
Compare
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/kg-lexical-html-renderer/package.json (1)
10-16:⚠️ Potential issue | 🟠 Major
exportsnow hides previously reachable deep subpaths.Lines 10-16 only export
".". If consumers currently deep-import anything underbuild/transformers/*orbuild/utils/*, this change turns those imports intoERR_PACKAGE_PATH_NOT_EXPORTED. Either keep those subpaths exported or treat this as a major-version break.#!/bin/bash set -euo pipefail echo "== current exports map ==" sed -n '7,16p' packages/kg-lexical-html-renderer/package.json echo latest="$(curl -s https://registry.npmjs.org/%40tryghost%2Fkg-lexical-html-renderer | jq -r '."dist-tags".latest')" echo "latest published version: ${latest}" echo echo "== published deep paths in latest release ==" curl -sL "https://unpkg.com/%40tryghost/kg-lexical-html-renderer@${latest}/?meta" \ | jq -r '.files[].path' \ | rg '^/build/(transformers|utils)/' echo echo "== in-repo deep import references ==" rg -nP '@tryghost/kg-lexical-html-renderer/' .🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/package.json` around lines 10 - 16, The package.json exports map currently only exposes "." which hides previously reachable deep subpaths (e.g., build/transformers/* and build/utils/*) and will break consumers that deep-import them; update the "exports" field in packages/kg-lexical-html-renderer/package.json to explicitly re-export those subpaths (for example add entries for "./build/transformers/*" and "./build/utils/*" mapping import/require/types to the corresponding build/esm and build/cjs files) or, if you intend to remove deep imports, document this as a breaking change and bump the major version; locate the "exports" object in package.json to add the subpath entries or perform the version bump and changelog update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/kg-lexical-html-renderer/package.json`:
- Around line 10-16: The package.json exports map currently only exposes "."
which hides previously reachable deep subpaths (e.g., build/transformers/* and
build/utils/*) and will break consumers that deep-import them; update the
"exports" field in packages/kg-lexical-html-renderer/package.json to explicitly
re-export those subpaths (for example add entries for "./build/transformers/*"
and "./build/utils/*" mapping import/require/types to the corresponding
build/esm and build/cjs files) or, if you intend to remove deep imports,
document this as a breaking change and bump the major version; locate the
"exports" object in package.json to add the subpath entries or perform the
version bump and changelog update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6918de39-c0aa-4ffa-b385-8a096f6cf6f9
📒 Files selected for processing (8)
packages/kg-default-nodes/src/KoenigDecoratorNode.tspackages/kg-lexical-html-renderer/eslint.config.mjspackages/kg-lexical-html-renderer/lib/index.tspackages/kg-lexical-html-renderer/lib/kg-default-nodes.d.tspackages/kg-lexical-html-renderer/lib/kg-utils.d.tspackages/kg-lexical-html-renderer/lib/transformers/index.tspackages/kg-lexical-html-renderer/package.jsonpackages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.ts
💤 Files with no reviewable changes (4)
- packages/kg-lexical-html-renderer/lib/index.ts
- packages/kg-lexical-html-renderer/lib/kg-utils.d.ts
- packages/kg-lexical-html-renderer/lib/transformers/index.ts
- packages/kg-lexical-html-renderer/lib/kg-default-nodes.d.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.ts
3d7be43 to
c0ea46a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/kg-default-nodes/src/nodes/audio/audio-renderer.ts (1)
26-26: NarrowAudioExportDOMOutputto actual variants returned.Line 26 is currently broader than runtime behavior. Restricting to
'inner' | 'outer'keeps the public contract precise.🎯 Suggested type narrowing
-export type AudioExportDOMOutput = ExportDOMOutput; +export type AudioExportDOMOutput = ExportDOMOutput<'inner' | 'outer'>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-default-nodes/src/nodes/audio/audio-renderer.ts` at line 26, AudioExportDOMOutput is currently aliased to the broad ExportDOMOutput; narrow it to only the variants actually produced at runtime by replacing the alias with a union type limited to 'inner' | 'outer' so the public type matches behavior. Update the declaration for AudioExportDOMOutput (referencing the existing ExportDOMOutput alias) to export type AudioExportDOMOutput = 'inner' | 'outer' and run type checks to ensure all usages (e.g., any functions or components expecting AudioExportDOMOutput) still satisfy the narrower union.packages/kg-default-nodes/src/nodes/email/email-renderer.ts (1)
13-13: Simplify duplicate return union.Line 13 can be reduced to a single type because
EmptyContainerOutputalready resolves toExportDOMOutput<'inner'>.♻️ Suggested cleanup
-export function renderEmailNode(node: EmailNodeData, options: RenderOptions = {}): EmptyContainerOutput | ExportDOMOutput<'inner'> { +export function renderEmailNode(node: EmailNodeData, options: RenderOptions = {}): EmptyContainerOutput {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-default-nodes/src/nodes/email/email-renderer.ts` at line 13, The return type on renderEmailNode can be simplified by removing the redundant union with EmptyContainerOutput—update the function signature of renderEmailNode(node: EmailNodeData, options: RenderOptions = {}) to return only ExportDOMOutput<'inner'> (since EmptyContainerOutput already resolves to that); adjust any related type annotations or imports if necessary to ensure the signature compiles.packages/kg-default-nodes/test/generate-decorator-node.test.ts (1)
28-36: Optional: consolidate repeatedHTMLElementassertion helpers across node tests.You now have near-identical helpers in multiple test files; moving one shared helper into
test-utilswould reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-default-nodes/test/generate-decorator-node.test.ts` around lines 28 - 36, Tests duplicate the helper expectHtmlElement(ExportDOMOutput) that asserts an HTML element; consolidate by extracting this function into a shared test-utils module and updating tests to import it. Create a new exported helper (e.g., expectHtmlElement) in the test-utils file that takes ExportDOMOutput, performs the same null/'outerHTML' check and returns the element; update all tests (including generate-decorator-node.test.ts) to import expectHtmlElement from test-utils instead of declaring it locally; ensure the helper is exported and any necessary types (ExportDOMOutput) are re-exported or imported in the test-utils module so existing tests compile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/kg-default-nodes/src/utils/get-first-html-element.ts`:
- Around line 4-5: The function get-first-html-element currently only inspects
the first child element and its namespace, allowing multiple sibling elements to
pass silently; update the logic to enforce single-root semantics by checking
container.childElementCount === 1 and throwing the same Error using the existing
context when the count is not exactly one, then proceed to validate the single
element's namespace (element.namespaceURI) as before; reference the local
variables element and context (and the enclosing function getFirstHtmlElement)
when making this change so the check occurs before returning the element.
In `@packages/kg-lexical-html-renderer/README.md`:
- Around line 25-29: The README example incorrectly uses top-level await with
CommonJS require(); update the example to run the asynchronous call inside an
async function or IIFE: import/create the renderer using const
{LexicalHTMLRenderer} = require('@tryghost/kg-lexical-html-renderer'); const
renderer = new LexicalHTMLRenderer(); then call renderer.render(lexicalState)
from within an async function (or immediately-invoked async function) so the
await is valid; ensure the sample references LexicalHTMLRenderer and its render
method so readers can locate the change.
---
Nitpick comments:
In `@packages/kg-default-nodes/src/nodes/audio/audio-renderer.ts`:
- Line 26: AudioExportDOMOutput is currently aliased to the broad
ExportDOMOutput; narrow it to only the variants actually produced at runtime by
replacing the alias with a union type limited to 'inner' | 'outer' so the public
type matches behavior. Update the declaration for AudioExportDOMOutput
(referencing the existing ExportDOMOutput alias) to export type
AudioExportDOMOutput = 'inner' | 'outer' and run type checks to ensure all
usages (e.g., any functions or components expecting AudioExportDOMOutput) still
satisfy the narrower union.
In `@packages/kg-default-nodes/src/nodes/email/email-renderer.ts`:
- Line 13: The return type on renderEmailNode can be simplified by removing the
redundant union with EmptyContainerOutput—update the function signature of
renderEmailNode(node: EmailNodeData, options: RenderOptions = {}) to return only
ExportDOMOutput<'inner'> (since EmptyContainerOutput already resolves to that);
adjust any related type annotations or imports if necessary to ensure the
signature compiles.
In `@packages/kg-default-nodes/test/generate-decorator-node.test.ts`:
- Around line 28-36: Tests duplicate the helper
expectHtmlElement(ExportDOMOutput) that asserts an HTML element; consolidate by
extracting this function into a shared test-utils module and updating tests to
import it. Create a new exported helper (e.g., expectHtmlElement) in the
test-utils file that takes ExportDOMOutput, performs the same null/'outerHTML'
check and returns the element; update all tests (including
generate-decorator-node.test.ts) to import expectHtmlElement from test-utils
instead of declaring it locally; ensure the helper is exported and any necessary
types (ExportDOMOutput) are re-exported or imported in the test-utils module so
existing tests compile.
🪄 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: be8650ad-a365-444b-9b06-55376ab41567
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (54)
packages/kg-default-nodes/src/KoenigDecoratorNode.tspackages/kg-default-nodes/src/export-dom.tspackages/kg-default-nodes/src/generate-decorator-node.tspackages/kg-default-nodes/src/nodes/audio/audio-renderer.tspackages/kg-default-nodes/src/nodes/call-to-action/calltoaction-renderer.tspackages/kg-default-nodes/src/nodes/email/email-renderer.tspackages/kg-default-nodes/src/nodes/file/file-renderer.tspackages/kg-default-nodes/src/nodes/header/renderers/v2/header-renderer.tspackages/kg-default-nodes/src/nodes/html/html-renderer.tspackages/kg-default-nodes/src/nodes/markdown/markdown-renderer.tspackages/kg-default-nodes/src/nodes/paywall/paywall-renderer.tspackages/kg-default-nodes/src/nodes/product/product-renderer.tspackages/kg-default-nodes/src/nodes/signup/signup-renderer.tspackages/kg-default-nodes/src/nodes/toggle/toggle-renderer.tspackages/kg-default-nodes/src/nodes/video/video-renderer.tspackages/kg-default-nodes/src/utils/get-first-html-element.tspackages/kg-default-nodes/src/utils/render-empty-container.tspackages/kg-default-nodes/src/utils/visibility.tspackages/kg-default-nodes/src/visibility.tspackages/kg-default-nodes/test/generate-decorator-node.test.tspackages/kg-default-nodes/test/nodes/audio.test.tspackages/kg-default-nodes/test/nodes/paywall.test.tspackages/kg-default-nodes/test/utils/visibility.test.tspackages/kg-lexical-html-renderer/README.mdpackages/kg-lexical-html-renderer/eslint.config.mjspackages/kg-lexical-html-renderer/lib/index.tspackages/kg-lexical-html-renderer/lib/transformers/index.tspackages/kg-lexical-html-renderer/package.jsonpackages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.tspackages/kg-lexical-html-renderer/src/convert-to-html-string.tspackages/kg-lexical-html-renderer/src/get-dynamic-data-nodes.tspackages/kg-lexical-html-renderer/src/index.tspackages/kg-lexical-html-renderer/src/transformers/element/aside.tspackages/kg-lexical-html-renderer/src/transformers/element/blockquote.tspackages/kg-lexical-html-renderer/src/transformers/element/heading.tspackages/kg-lexical-html-renderer/src/transformers/element/list.tspackages/kg-lexical-html-renderer/src/transformers/element/paragraph.tspackages/kg-lexical-html-renderer/src/transformers/index.tspackages/kg-lexical-html-renderer/src/types.tspackages/kg-lexical-html-renderer/src/utils/TextContent.tspackages/kg-lexical-html-renderer/src/utils/generate-id.tspackages/kg-lexical-html-renderer/test/cards.test.tspackages/kg-lexical-html-renderer/test/headings.test.tspackages/kg-lexical-html-renderer/test/links.test.tspackages/kg-lexical-html-renderer/test/lists.test.tspackages/kg-lexical-html-renderer/test/quotes.test.tspackages/kg-lexical-html-renderer/test/render.test.tspackages/kg-lexical-html-renderer/test/text-formats.test.tspackages/kg-lexical-html-renderer/test/utils/index.tspackages/kg-lexical-html-renderer/test/utils/overrides.tspackages/kg-lexical-html-renderer/test/utils/should-render.tspackages/kg-lexical-html-renderer/tsconfig.cjs.jsonpackages/kg-lexical-html-renderer/tsconfig.jsonpackages/kg-lexical-html-renderer/tsconfig.test.json
💤 Files with no reviewable changes (3)
- packages/kg-default-nodes/src/visibility.ts
- packages/kg-lexical-html-renderer/lib/index.ts
- packages/kg-lexical-html-renderer/lib/transformers/index.ts
✅ Files skipped from review due to trivial changes (10)
- packages/kg-lexical-html-renderer/test/lists.test.ts
- packages/kg-lexical-html-renderer/src/utils/TextContent.ts
- packages/kg-lexical-html-renderer/test/text-formats.test.ts
- packages/kg-lexical-html-renderer/test/quotes.test.ts
- packages/kg-lexical-html-renderer/test/headings.test.ts
- packages/kg-lexical-html-renderer/src/index.ts
- packages/kg-lexical-html-renderer/tsconfig.cjs.json
- packages/kg-lexical-html-renderer/tsconfig.test.json
- packages/kg-lexical-html-renderer/src/transformers/index.ts
- packages/kg-lexical-html-renderer/test/links.test.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/kg-lexical-html-renderer/src/utils/generate-id.ts
- packages/kg-lexical-html-renderer/test/render.test.ts
- packages/kg-lexical-html-renderer/test/utils/overrides.ts
- packages/kg-lexical-html-renderer/test/cards.test.ts
- packages/kg-lexical-html-renderer/test/utils/index.ts
- packages/kg-lexical-html-renderer/src/transformers/element/list.ts
- packages/kg-lexical-html-renderer/src/transformers/element/heading.ts
- packages/kg-lexical-html-renderer/src/transformers/element/paragraph.ts
- packages/kg-lexical-html-renderer/tsconfig.json
- packages/kg-lexical-html-renderer/src/transformers/element/blockquote.ts
- packages/kg-lexical-html-renderer/test/utils/should-render.ts
- packages/kg-lexical-html-renderer/package.json
c0ea46a to
ded4b86
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/kg-lexical-html-renderer/src/convert-to-html-string.ts (1)
40-45: Optional: extractvaluerendering into a helper for symmetry.
innerand default already use helpers; doing the same forvaluewould keep the switch fully declarative and easier to test.♻️ Suggested refactor
switch (type) { case 'inner': return getElementInnerHTML(element); case 'value': - if (element && 'value' in element && typeof element.value === 'string') { - return element.value; - } - - return ''; + return getElementValue(element); default: return getElementOuterHTML(element); } @@ function getElementOuterHTML(element: HTMLElement | Text | null): string { if (element && 'outerHTML' in element) { return element.outerHTML; } return ''; } + +function getElementValue(element: HTMLElement | Text | null): string { + if (element && 'value' in element && typeof element.value === 'string') { + return element.value; + } + + return ''; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/src/convert-to-html-string.ts` around lines 40 - 45, Extract the inline 'value' case logic into a small helper (e.g., renderValue(element): string) to match the existing helpers used for 'inner' and default branches; move the check (element && 'value' in element && typeof element.value === 'string') into renderValue and return element.value or '' accordingly, then replace the switch's 'value' branch with a call to renderValue(element) so the switch remains fully declarative and easier to unit test.packages/kg-lexical-html-renderer/package.json (1)
19-24: Consider extracting the repeated build steps.The build sequence
yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.jsonis duplicated verbatim inbuild,prepare, andpretest. This could be extracted into a single script (e.g.,build:all) that the others invoke.♻️ Proposed refactor
"scripts": { "dev": "tsc --watch --preserveWatchOutput --sourceMap", "clean": "rm -rf build tsconfig.tsbuildinfo", - "build": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", - "prepare": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", - "pretest": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json && tsc -p tsconfig.test.json", + "build": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", + "prepare": "yarn build", + "pretest": "yarn build && tsc -p tsconfig.test.json",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/package.json` around lines 19 - 24, Extract the repeated sequence in package.json into a new script (e.g., "build:all") and have "build", "prepare", and "pretest" call that script; specifically, create a new npm script named "build:all" that runs the common pipeline (yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{"type":"module"}' > build/esm/package.json) and update the existing "build", "prepare", and "pretest" scripts to invoke "yarn build:all" (keeping any additional steps like tsc -p tsconfig.test.json in "pretest" after calling the shared script).packages/kg-default-nodes/src/nodes/audio/audio-renderer.ts (1)
26-26: Consider narrowingAudioExportDOMOutputto emitted variants only.At Line 26, aliasing to full
ExportDOMOutputallows'html' | 'value'variants that this renderer does not emit, which dilutes type safety for consumers.Proposed type refinement
-export type AudioExportDOMOutput = ExportDOMOutput; +export type AudioExportDOMOutput = ExportDOMOutput<'outer'> | ExportDOMOutput<'inner'>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-default-nodes/src/nodes/audio/audio-renderer.ts` at line 26, AudioExportDOMOutput currently aliases the entire ExportDOMOutput (allowing 'html'|'value' variants) which this renderer never emits; change AudioExportDOMOutput to a narrowed type that only includes the specific variants this audio renderer emits (e.g., a union or mapped subset of ExportDOMOutput containing the audio/media output shape). Update the alias for AudioExportDOMOutput in audio-renderer.ts to reference only those emitted keys (using an explicit union of the emitted variant names or a Pick/Lookup on ExportDOMOutput) so consumers get correct type safety for functions/classes like the audio renderer export logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/kg-default-nodes/src/nodes/audio/audio-renderer.ts`:
- Line 26: AudioExportDOMOutput currently aliases the entire ExportDOMOutput
(allowing 'html'|'value' variants) which this renderer never emits; change
AudioExportDOMOutput to a narrowed type that only includes the specific variants
this audio renderer emits (e.g., a union or mapped subset of ExportDOMOutput
containing the audio/media output shape). Update the alias for
AudioExportDOMOutput in audio-renderer.ts to reference only those emitted keys
(using an explicit union of the emitted variant names or a Pick/Lookup on
ExportDOMOutput) so consumers get correct type safety for functions/classes like
the audio renderer export logic.
In `@packages/kg-lexical-html-renderer/package.json`:
- Around line 19-24: Extract the repeated sequence in package.json into a new
script (e.g., "build:all") and have "build", "prepare", and "pretest" call that
script; specifically, create a new npm script named "build:all" that runs the
common pipeline (yarn clean && tsc && tsc -p tsconfig.cjs.json && echo
'{"type":"module"}' > build/esm/package.json) and update the existing "build",
"prepare", and "pretest" scripts to invoke "yarn build:all" (keeping any
additional steps like tsc -p tsconfig.test.json in "pretest" after calling the
shared script).
In `@packages/kg-lexical-html-renderer/src/convert-to-html-string.ts`:
- Around line 40-45: Extract the inline 'value' case logic into a small helper
(e.g., renderValue(element): string) to match the existing helpers used for
'inner' and default branches; move the check (element && 'value' in element &&
typeof element.value === 'string') into renderValue and return element.value or
'' accordingly, then replace the switch's 'value' branch with a call to
renderValue(element) so the switch remains fully declarative and easier to unit
test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb1e7b9b-c73b-41ad-bb6e-8d03639af9f0
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (54)
packages/kg-default-nodes/src/KoenigDecoratorNode.tspackages/kg-default-nodes/src/export-dom.tspackages/kg-default-nodes/src/generate-decorator-node.tspackages/kg-default-nodes/src/nodes/audio/audio-renderer.tspackages/kg-default-nodes/src/nodes/call-to-action/calltoaction-renderer.tspackages/kg-default-nodes/src/nodes/email/email-renderer.tspackages/kg-default-nodes/src/nodes/file/file-renderer.tspackages/kg-default-nodes/src/nodes/header/renderers/v2/header-renderer.tspackages/kg-default-nodes/src/nodes/html/html-renderer.tspackages/kg-default-nodes/src/nodes/markdown/markdown-renderer.tspackages/kg-default-nodes/src/nodes/paywall/paywall-renderer.tspackages/kg-default-nodes/src/nodes/product/product-renderer.tspackages/kg-default-nodes/src/nodes/signup/signup-renderer.tspackages/kg-default-nodes/src/nodes/toggle/toggle-renderer.tspackages/kg-default-nodes/src/nodes/video/video-renderer.tspackages/kg-default-nodes/src/utils/get-first-html-element.tspackages/kg-default-nodes/src/utils/render-empty-container.tspackages/kg-default-nodes/src/utils/visibility.tspackages/kg-default-nodes/src/visibility.tspackages/kg-default-nodes/test/generate-decorator-node.test.tspackages/kg-default-nodes/test/nodes/audio.test.tspackages/kg-default-nodes/test/nodes/paywall.test.tspackages/kg-default-nodes/test/utils/visibility.test.tspackages/kg-lexical-html-renderer/README.mdpackages/kg-lexical-html-renderer/eslint.config.mjspackages/kg-lexical-html-renderer/lib/index.tspackages/kg-lexical-html-renderer/lib/transformers/index.tspackages/kg-lexical-html-renderer/package.jsonpackages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.tspackages/kg-lexical-html-renderer/src/convert-to-html-string.tspackages/kg-lexical-html-renderer/src/get-dynamic-data-nodes.tspackages/kg-lexical-html-renderer/src/index.tspackages/kg-lexical-html-renderer/src/transformers/element/aside.tspackages/kg-lexical-html-renderer/src/transformers/element/blockquote.tspackages/kg-lexical-html-renderer/src/transformers/element/heading.tspackages/kg-lexical-html-renderer/src/transformers/element/list.tspackages/kg-lexical-html-renderer/src/transformers/element/paragraph.tspackages/kg-lexical-html-renderer/src/transformers/index.tspackages/kg-lexical-html-renderer/src/types.tspackages/kg-lexical-html-renderer/src/utils/TextContent.tspackages/kg-lexical-html-renderer/src/utils/generate-id.tspackages/kg-lexical-html-renderer/test/cards.test.tspackages/kg-lexical-html-renderer/test/headings.test.tspackages/kg-lexical-html-renderer/test/links.test.tspackages/kg-lexical-html-renderer/test/lists.test.tspackages/kg-lexical-html-renderer/test/quotes.test.tspackages/kg-lexical-html-renderer/test/render.test.tspackages/kg-lexical-html-renderer/test/text-formats.test.tspackages/kg-lexical-html-renderer/test/utils/index.tspackages/kg-lexical-html-renderer/test/utils/overrides.tspackages/kg-lexical-html-renderer/test/utils/should-render.tspackages/kg-lexical-html-renderer/tsconfig.cjs.jsonpackages/kg-lexical-html-renderer/tsconfig.jsonpackages/kg-lexical-html-renderer/tsconfig.test.json
💤 Files with no reviewable changes (3)
- packages/kg-lexical-html-renderer/lib/index.ts
- packages/kg-default-nodes/src/visibility.ts
- packages/kg-lexical-html-renderer/lib/transformers/index.ts
✅ Files skipped from review due to trivial changes (17)
- packages/kg-default-nodes/src/nodes/file/file-renderer.ts
- packages/kg-lexical-html-renderer/test/headings.test.ts
- packages/kg-lexical-html-renderer/test/lists.test.ts
- packages/kg-lexical-html-renderer/src/utils/TextContent.ts
- packages/kg-default-nodes/src/utils/render-empty-container.ts
- packages/kg-default-nodes/src/utils/get-first-html-element.ts
- packages/kg-lexical-html-renderer/test/quotes.test.ts
- packages/kg-default-nodes/src/nodes/paywall/paywall-renderer.ts
- packages/kg-lexical-html-renderer/test/links.test.ts
- packages/kg-lexical-html-renderer/src/index.ts
- packages/kg-default-nodes/src/nodes/email/email-renderer.ts
- packages/kg-lexical-html-renderer/test/utils/overrides.ts
- packages/kg-lexical-html-renderer/tsconfig.test.json
- packages/kg-lexical-html-renderer/src/types.ts
- packages/kg-lexical-html-renderer/tsconfig.cjs.json
- packages/kg-lexical-html-renderer/src/transformers/element/list.ts
- packages/kg-default-nodes/src/nodes/markdown/markdown-renderer.ts
🚧 Files skipped from review as they are similar to previous changes (25)
- packages/kg-default-nodes/src/nodes/video/video-renderer.ts
- packages/kg-lexical-html-renderer/README.md
- packages/kg-default-nodes/src/nodes/signup/signup-renderer.ts
- packages/kg-default-nodes/src/nodes/toggle/toggle-renderer.ts
- packages/kg-lexical-html-renderer/test/text-formats.test.ts
- packages/kg-default-nodes/test/generate-decorator-node.test.ts
- packages/kg-default-nodes/src/nodes/header/renderers/v2/header-renderer.ts
- packages/kg-default-nodes/test/nodes/paywall.test.ts
- packages/kg-default-nodes/src/nodes/product/product-renderer.ts
- packages/kg-default-nodes/src/generate-decorator-node.ts
- packages/kg-lexical-html-renderer/test/render.test.ts
- packages/kg-lexical-html-renderer/src/transformers/element/aside.ts
- packages/kg-lexical-html-renderer/src/transformers/element/blockquote.ts
- packages/kg-default-nodes/src/nodes/html/html-renderer.ts
- packages/kg-lexical-html-renderer/src/utils/generate-id.ts
- packages/kg-lexical-html-renderer/test/cards.test.ts
- packages/kg-lexical-html-renderer/src/get-dynamic-data-nodes.ts
- packages/kg-lexical-html-renderer/src/transformers/index.ts
- packages/kg-lexical-html-renderer/src/transformers/element/heading.ts
- packages/kg-lexical-html-renderer/test/utils/index.ts
- packages/kg-default-nodes/src/KoenigDecoratorNode.ts
- packages/kg-lexical-html-renderer/test/utils/should-render.ts
- packages/kg-default-nodes/src/export-dom.ts
- packages/kg-lexical-html-renderer/src/transformers/element/paragraph.ts
- packages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.ts
ded4b86 to
11d62dc
Compare
no issue This commit moves kg-lexical-html-renderer from lib/ to src/, migrates its build and tests to TypeScript/ESM, and fixes the migration fallout in published artifacts, runtime loading, and callback typing. It also tightens the shared exportDOM and Koenig card typing in kg-default-nodes because this renderer migration depends on those cross-package contracts compiling cleanly against Lexical.
11d62dc to
a879a23
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/kg-lexical-html-renderer/src/convert-to-html-string.ts (1)
93-106: Derive these helper parameter types from the export contract instead of re-declaring them.
HTMLElement | Text | nullis now duplicated here, while the real shape comes from the Lexical/@tryghost/kg-default-nodesDOM export types. If that contract changes again, this file drifts out of sync first. PreferExportDOMOutput['element'](or the package’s exported element alias) for both helpers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/src/convert-to-html-string.ts` around lines 93 - 106, The helper functions getElementInnerHTML and getElementOuterHTML duplicate the parameter type (HTMLElement | Text | null); change their parameter type to the exported DOM element type from the package (e.g. ExportDOMOutput['element'] or the package’s exported element alias) and update imports accordingly so the helpers derive their type from the public export contract rather than re-declaring it.packages/kg-lexical-html-renderer/package.json (2)
44-44:jsdomis listed in bothdevDependenciesanddependencies.The package
jsdomat version29.0.2appears in bothdevDependencies(line 44) anddependencies(line 62). ThedevDependenciesentry is redundant since npm resolves fromdependencieswhen both are present.♻️ Proposed fix to remove redundant devDependency
"devDependencies": { "@eslint/js": "9.39.4", "@types/jsdom": "21.1.7", "@types/mocha": "10.0.10", "@types/should": "13.0.0", "@types/sinon": "21.0.0", "c8": "11.0.0", - "jsdom": "29.0.2", "mocha": "11.7.5",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/package.json` at line 44, Remove the redundant devDependency entry for "jsdom": "29.0.2" so it only appears once under dependencies; open package.json, locate the devDependencies block containing "jsdom": "29.0.2" and delete that line (leaving the existing "jsdom": "29.0.2" in dependencies intact), then run npm install / yarn install to update lockfiles.
19-22: Consider deduplicating the build scripts.The
preparescript is identical tobuild, andpretestrepeats the entire build chain before adding test compilation. This duplication means changes to the build process require updating multiple scripts.♻️ Proposed refactor to reduce duplication
"clean": "rm -rf build tsconfig.tsbuildinfo", "build": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", - "prepare": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", - "pretest": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json && tsc -p tsconfig.test.json", + "prepare": "yarn build", + "pretest": "yarn build && tsc -p tsconfig.test.json",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/package.json` around lines 19 - 22, The build commands are duplicated across the "build", "prepare", and "pretest" npm scripts; extract the common sequence (rm -rf build tsconfig.tsbuildinfo; tsc; tsc -p tsconfig.cjs.json; echo '{"type":"module"}' > build/esm/package.json) into a single reusable script (e.g., "build:all"), update "build" and "prepare" to invoke that new script, and change "pretest" to run the shared build script followed by only the test compilation step (tsc -p tsconfig.test.json) so the build logic is maintained in one place and not repeated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/kg-lexical-html-renderer/package.json`:
- Line 44: Remove the redundant devDependency entry for "jsdom": "29.0.2" so it
only appears once under dependencies; open package.json, locate the
devDependencies block containing "jsdom": "29.0.2" and delete that line (leaving
the existing "jsdom": "29.0.2" in dependencies intact), then run npm install /
yarn install to update lockfiles.
- Around line 19-22: The build commands are duplicated across the "build",
"prepare", and "pretest" npm scripts; extract the common sequence (rm -rf build
tsconfig.tsbuildinfo; tsc; tsc -p tsconfig.cjs.json; echo '{"type":"module"}' >
build/esm/package.json) into a single reusable script (e.g., "build:all"),
update "build" and "prepare" to invoke that new script, and change "pretest" to
run the shared build script followed by only the test compilation step (tsc -p
tsconfig.test.json) so the build logic is maintained in one place and not
repeated.
In `@packages/kg-lexical-html-renderer/src/convert-to-html-string.ts`:
- Around line 93-106: The helper functions getElementInnerHTML and
getElementOuterHTML duplicate the parameter type (HTMLElement | Text | null);
change their parameter type to the exported DOM element type from the package
(e.g. ExportDOMOutput['element'] or the package’s exported element alias) and
update imports accordingly so the helpers derive their type from the public
export contract rather than re-declaring it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58c64b9b-1ad4-40b3-8b75-fe3dc04beac0
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (54)
packages/kg-default-nodes/src/KoenigDecoratorNode.tspackages/kg-default-nodes/src/export-dom.tspackages/kg-default-nodes/src/generate-decorator-node.tspackages/kg-default-nodes/src/nodes/audio/audio-renderer.tspackages/kg-default-nodes/src/nodes/call-to-action/calltoaction-renderer.tspackages/kg-default-nodes/src/nodes/email/email-renderer.tspackages/kg-default-nodes/src/nodes/file/file-renderer.tspackages/kg-default-nodes/src/nodes/header/renderers/v2/header-renderer.tspackages/kg-default-nodes/src/nodes/html/html-renderer.tspackages/kg-default-nodes/src/nodes/markdown/markdown-renderer.tspackages/kg-default-nodes/src/nodes/paywall/paywall-renderer.tspackages/kg-default-nodes/src/nodes/product/product-renderer.tspackages/kg-default-nodes/src/nodes/signup/signup-renderer.tspackages/kg-default-nodes/src/nodes/toggle/toggle-renderer.tspackages/kg-default-nodes/src/nodes/video/video-renderer.tspackages/kg-default-nodes/src/utils/get-first-html-element.tspackages/kg-default-nodes/src/utils/render-empty-container.tspackages/kg-default-nodes/src/utils/visibility.tspackages/kg-default-nodes/src/visibility.tspackages/kg-default-nodes/test/generate-decorator-node.test.tspackages/kg-default-nodes/test/nodes/audio.test.tspackages/kg-default-nodes/test/nodes/paywall.test.tspackages/kg-default-nodes/test/utils/visibility.test.tspackages/kg-lexical-html-renderer/README.mdpackages/kg-lexical-html-renderer/eslint.config.mjspackages/kg-lexical-html-renderer/lib/index.tspackages/kg-lexical-html-renderer/lib/transformers/index.tspackages/kg-lexical-html-renderer/package.jsonpackages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.tspackages/kg-lexical-html-renderer/src/convert-to-html-string.tspackages/kg-lexical-html-renderer/src/get-dynamic-data-nodes.tspackages/kg-lexical-html-renderer/src/index.tspackages/kg-lexical-html-renderer/src/transformers/element/aside.tspackages/kg-lexical-html-renderer/src/transformers/element/blockquote.tspackages/kg-lexical-html-renderer/src/transformers/element/heading.tspackages/kg-lexical-html-renderer/src/transformers/element/list.tspackages/kg-lexical-html-renderer/src/transformers/element/paragraph.tspackages/kg-lexical-html-renderer/src/transformers/index.tspackages/kg-lexical-html-renderer/src/types.tspackages/kg-lexical-html-renderer/src/utils/TextContent.tspackages/kg-lexical-html-renderer/src/utils/generate-id.tspackages/kg-lexical-html-renderer/test/cards.test.tspackages/kg-lexical-html-renderer/test/headings.test.tspackages/kg-lexical-html-renderer/test/links.test.tspackages/kg-lexical-html-renderer/test/lists.test.tspackages/kg-lexical-html-renderer/test/quotes.test.tspackages/kg-lexical-html-renderer/test/render.test.tspackages/kg-lexical-html-renderer/test/text-formats.test.tspackages/kg-lexical-html-renderer/test/utils/index.tspackages/kg-lexical-html-renderer/test/utils/overrides.tspackages/kg-lexical-html-renderer/test/utils/should-render.tspackages/kg-lexical-html-renderer/tsconfig.cjs.jsonpackages/kg-lexical-html-renderer/tsconfig.jsonpackages/kg-lexical-html-renderer/tsconfig.test.json
💤 Files with no reviewable changes (3)
- packages/kg-lexical-html-renderer/lib/index.ts
- packages/kg-default-nodes/src/visibility.ts
- packages/kg-lexical-html-renderer/lib/transformers/index.ts
✅ Files skipped from review due to trivial changes (16)
- packages/kg-lexical-html-renderer/test/text-formats.test.ts
- packages/kg-lexical-html-renderer/test/headings.test.ts
- packages/kg-lexical-html-renderer/test/quotes.test.ts
- packages/kg-lexical-html-renderer/test/lists.test.ts
- packages/kg-default-nodes/src/nodes/file/file-renderer.ts
- packages/kg-default-nodes/src/utils/render-empty-container.ts
- packages/kg-lexical-html-renderer/src/index.ts
- packages/kg-lexical-html-renderer/test/links.test.ts
- packages/kg-lexical-html-renderer/tsconfig.test.json
- packages/kg-default-nodes/test/generate-decorator-node.test.ts
- packages/kg-lexical-html-renderer/src/types.ts
- packages/kg-lexical-html-renderer/src/utils/TextContent.ts
- packages/kg-lexical-html-renderer/tsconfig.cjs.json
- packages/kg-default-nodes/src/nodes/markdown/markdown-renderer.ts
- packages/kg-default-nodes/src/utils/get-first-html-element.ts
- packages/kg-default-nodes/src/nodes/header/renderers/v2/header-renderer.ts
🚧 Files skipped from review as they are similar to previous changes (23)
- packages/kg-default-nodes/src/nodes/product/product-renderer.ts
- packages/kg-default-nodes/src/nodes/toggle/toggle-renderer.ts
- packages/kg-default-nodes/src/nodes/signup/signup-renderer.ts
- packages/kg-lexical-html-renderer/src/utils/generate-id.ts
- packages/kg-lexical-html-renderer/README.md
- packages/kg-default-nodes/test/nodes/paywall.test.ts
- packages/kg-lexical-html-renderer/test/render.test.ts
- packages/kg-default-nodes/src/nodes/paywall/paywall-renderer.ts
- packages/kg-lexical-html-renderer/test/cards.test.ts
- packages/kg-lexical-html-renderer/test/utils/index.ts
- packages/kg-default-nodes/src/generate-decorator-node.ts
- packages/kg-lexical-html-renderer/src/transformers/element/blockquote.ts
- packages/kg-default-nodes/src/nodes/email/email-renderer.ts
- packages/kg-lexical-html-renderer/src/transformers/element/list.ts
- packages/kg-lexical-html-renderer/test/utils/overrides.ts
- packages/kg-lexical-html-renderer/src/get-dynamic-data-nodes.ts
- packages/kg-default-nodes/src/nodes/html/html-renderer.ts
- packages/kg-default-nodes/src/export-dom.ts
- packages/kg-default-nodes/src/KoenigDecoratorNode.ts
- packages/kg-lexical-html-renderer/src/transformers/index.ts
- packages/kg-lexical-html-renderer/src/transformers/element/paragraph.ts
- packages/kg-default-nodes/src/nodes/call-to-action/calltoaction-renderer.ts
- packages/kg-lexical-html-renderer/test/utils/should-render.ts
There was a problem hiding this comment.
🧹 Nitpick comments (4)
packages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.ts (1)
82-84: Remove unnecessaryasyncfrom theeditor.updatecallback.The callback passed to
editor.update()is markedasync, but$convertToHtmlString()is synchronous. Lexical'seditor.update()executes callbacks synchronously and doesn't await returned Promises, so theasynckeyword is misleading and serves no purpose here.Suggested fix
- editor.update(async () => { + editor.update(() => { html = $convertToHtmlString(options); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.ts` around lines 82 - 84, The callback passed to editor.update unnecessarily uses the async keyword; remove async from the anonymous function so editor.update(() => { html = $convertToHtmlString(options); }) is used instead of editor.update(async () => { ... }), because $convertToHtmlString is synchronous and editor.update does not await promises; update the code around editor.update and the assignment to html accordingly.packages/kg-default-nodes/test/utils/visibility.test.ts (1)
250-251: Optional: cache narrowed elements per test block to reduce repetition.These blocks repeatedly call the same narrowing helper on the same value. Storing the narrowed element once improves readability and keeps assertions cleaner.
♻️ Example cleanup pattern
- getHTMLElement(result.element).tagName.should.eql('P'); - getHTMLElement(result.element).innerHTML.should.equal('testing'); + const element = getHTMLElement(result.element); + element.tagName.should.eql('P'); + element.innerHTML.should.equal('testing');Also applies to: 259-260, 268-270, 280-281, 289-290, 297-298, 310-311, 322-323, 334-335, 354-356
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-default-nodes/test/utils/visibility.test.ts` around lines 250 - 251, Tests repeatedly call getHTMLElement(result.element) for the same result; cache the narrowed HTMLElement once per test block to avoid repetition. Replace repeated calls like getHTMLElement(result.element).tagName and getHTMLElement(result.element).innerHTML with a local const (e.g., const el = getHTMLElement(result.element)) at the start of the test or assertion block, then use el.tagName and el.innerHTML for subsequent assertions; apply this pattern to all mentioned test blocks that reference result.element (lines comparing tagName and innerHTML).packages/kg-lexical-html-renderer/package.json (2)
38-51: Optional cleanup: avoid duplicate dependency declarations.
jsdomis declared in both runtime and dev dependencies; keeping it only independenciesis usually enough and avoids duplicate version maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/package.json` around lines 38 - 51, The package.json currently lists "jsdom" in both dependencies and devDependencies; remove the duplicate from devDependencies so "jsdom" is declared only once under dependencies, updating any consumer code/comments if necessary and ensuring the remaining declaration (the "jsdom" entry in dependencies) has the intended version; check the devDependencies block for any other duplicates and keep only one authoritative declaration.
20-22: De-duplicate repeated build pipeline scripts.Line 20, Line 21, and Line 22 repeat the same command chain. Extracting a shared script will reduce maintenance drift.
♻️ Suggested refactor
"scripts": { "dev": "tsc --watch --preserveWatchOutput --sourceMap", "clean": "rm -rf build tsconfig.tsbuildinfo", - "build": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", - "prepare": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", - "pretest": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json && tsc -p tsconfig.test.json", + "build:artifacts": "yarn clean && tsc && tsc -p tsconfig.cjs.json && echo '{\"type\":\"module\"}' > build/esm/package.json", + "build": "yarn build:artifacts", + "prepare": "yarn build:artifacts", + "pretest": "yarn build:artifacts && tsc -p tsconfig.test.json",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kg-lexical-html-renderer/package.json` around lines 20 - 22, The package.json scripts "build", "prepare", and "pretest" duplicate the same command chain; extract the common command sequence into a single shared script (e.g., "build:common" or "build:all") and have "build", "prepare", and "pretest" invoke that script (for example use "yarn build:all" in each) so the tsc + package.json echo logic is declared once; update the "build", "prepare", and "pretest" entries accordingly to call the new shared script.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/kg-default-nodes/test/utils/visibility.test.ts`:
- Around line 250-251: Tests repeatedly call getHTMLElement(result.element) for
the same result; cache the narrowed HTMLElement once per test block to avoid
repetition. Replace repeated calls like getHTMLElement(result.element).tagName
and getHTMLElement(result.element).innerHTML with a local const (e.g., const el
= getHTMLElement(result.element)) at the start of the test or assertion block,
then use el.tagName and el.innerHTML for subsequent assertions; apply this
pattern to all mentioned test blocks that reference result.element (lines
comparing tagName and innerHTML).
In `@packages/kg-lexical-html-renderer/package.json`:
- Around line 38-51: The package.json currently lists "jsdom" in both
dependencies and devDependencies; remove the duplicate from devDependencies so
"jsdom" is declared only once under dependencies, updating any consumer
code/comments if necessary and ensuring the remaining declaration (the "jsdom"
entry in dependencies) has the intended version; check the devDependencies block
for any other duplicates and keep only one authoritative declaration.
- Around line 20-22: The package.json scripts "build", "prepare", and "pretest"
duplicate the same command chain; extract the common command sequence into a
single shared script (e.g., "build:common" or "build:all") and have "build",
"prepare", and "pretest" invoke that script (for example use "yarn build:all" in
each) so the tsc + package.json echo logic is declared once; update the "build",
"prepare", and "pretest" entries accordingly to call the new shared script.
In `@packages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.ts`:
- Around line 82-84: The callback passed to editor.update unnecessarily uses the
async keyword; remove async from the anonymous function so editor.update(() => {
html = $convertToHtmlString(options); }) is used instead of editor.update(async
() => { ... }), because $convertToHtmlString is synchronous and editor.update
does not await promises; update the code around editor.update and the assignment
to html accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0d9cd9a-43cd-44df-abc8-3cf5e4450104
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (55)
packages/kg-default-nodes/package.jsonpackages/kg-default-nodes/src/KoenigDecoratorNode.tspackages/kg-default-nodes/src/export-dom.tspackages/kg-default-nodes/src/generate-decorator-node.tspackages/kg-default-nodes/src/nodes/audio/audio-renderer.tspackages/kg-default-nodes/src/nodes/call-to-action/calltoaction-renderer.tspackages/kg-default-nodes/src/nodes/email/email-renderer.tspackages/kg-default-nodes/src/nodes/file/file-renderer.tspackages/kg-default-nodes/src/nodes/header/renderers/v2/header-renderer.tspackages/kg-default-nodes/src/nodes/html/html-renderer.tspackages/kg-default-nodes/src/nodes/markdown/markdown-renderer.tspackages/kg-default-nodes/src/nodes/paywall/paywall-renderer.tspackages/kg-default-nodes/src/nodes/product/product-renderer.tspackages/kg-default-nodes/src/nodes/signup/signup-renderer.tspackages/kg-default-nodes/src/nodes/toggle/toggle-renderer.tspackages/kg-default-nodes/src/nodes/video/video-renderer.tspackages/kg-default-nodes/src/utils/get-first-html-element.tspackages/kg-default-nodes/src/utils/render-empty-container.tspackages/kg-default-nodes/src/utils/visibility.tspackages/kg-default-nodes/src/visibility.tspackages/kg-default-nodes/test/generate-decorator-node.test.tspackages/kg-default-nodes/test/nodes/audio.test.tspackages/kg-default-nodes/test/nodes/paywall.test.tspackages/kg-default-nodes/test/utils/visibility.test.tspackages/kg-lexical-html-renderer/README.mdpackages/kg-lexical-html-renderer/eslint.config.mjspackages/kg-lexical-html-renderer/lib/index.tspackages/kg-lexical-html-renderer/lib/transformers/index.tspackages/kg-lexical-html-renderer/package.jsonpackages/kg-lexical-html-renderer/src/LexicalHTMLRenderer.tspackages/kg-lexical-html-renderer/src/convert-to-html-string.tspackages/kg-lexical-html-renderer/src/get-dynamic-data-nodes.tspackages/kg-lexical-html-renderer/src/index.tspackages/kg-lexical-html-renderer/src/transformers/element/aside.tspackages/kg-lexical-html-renderer/src/transformers/element/blockquote.tspackages/kg-lexical-html-renderer/src/transformers/element/heading.tspackages/kg-lexical-html-renderer/src/transformers/element/list.tspackages/kg-lexical-html-renderer/src/transformers/element/paragraph.tspackages/kg-lexical-html-renderer/src/transformers/index.tspackages/kg-lexical-html-renderer/src/types.tspackages/kg-lexical-html-renderer/src/utils/TextContent.tspackages/kg-lexical-html-renderer/src/utils/generate-id.tspackages/kg-lexical-html-renderer/test/cards.test.tspackages/kg-lexical-html-renderer/test/headings.test.tspackages/kg-lexical-html-renderer/test/links.test.tspackages/kg-lexical-html-renderer/test/lists.test.tspackages/kg-lexical-html-renderer/test/quotes.test.tspackages/kg-lexical-html-renderer/test/render.test.tspackages/kg-lexical-html-renderer/test/text-formats.test.tspackages/kg-lexical-html-renderer/test/utils/index.tspackages/kg-lexical-html-renderer/test/utils/overrides.tspackages/kg-lexical-html-renderer/test/utils/should-render.tspackages/kg-lexical-html-renderer/tsconfig.cjs.jsonpackages/kg-lexical-html-renderer/tsconfig.jsonpackages/kg-lexical-html-renderer/tsconfig.test.json
💤 Files with no reviewable changes (3)
- packages/kg-default-nodes/src/visibility.ts
- packages/kg-lexical-html-renderer/lib/index.ts
- packages/kg-lexical-html-renderer/lib/transformers/index.ts
✅ Files skipped from review due to trivial changes (21)
- packages/kg-lexical-html-renderer/test/lists.test.ts
- packages/kg-lexical-html-renderer/test/headings.test.ts
- packages/kg-lexical-html-renderer/test/text-formats.test.ts
- packages/kg-lexical-html-renderer/test/quotes.test.ts
- packages/kg-default-nodes/src/utils/render-empty-container.ts
- packages/kg-lexical-html-renderer/src/index.ts
- packages/kg-default-nodes/package.json
- packages/kg-default-nodes/test/nodes/paywall.test.ts
- packages/kg-lexical-html-renderer/README.md
- packages/kg-lexical-html-renderer/src/utils/TextContent.ts
- packages/kg-lexical-html-renderer/test/links.test.ts
- packages/kg-default-nodes/test/generate-decorator-node.test.ts
- packages/kg-lexical-html-renderer/src/types.ts
- packages/kg-lexical-html-renderer/tsconfig.cjs.json
- packages/kg-default-nodes/src/nodes/header/renderers/v2/header-renderer.ts
- packages/kg-lexical-html-renderer/tsconfig.test.json
- packages/kg-default-nodes/src/nodes/toggle/toggle-renderer.ts
- packages/kg-lexical-html-renderer/test/utils/overrides.ts
- packages/kg-default-nodes/src/generate-decorator-node.ts
- packages/kg-lexical-html-renderer/src/transformers/index.ts
- packages/kg-lexical-html-renderer/test/utils/should-render.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- packages/kg-default-nodes/src/nodes/file/file-renderer.ts
- packages/kg-lexical-html-renderer/src/utils/generate-id.ts
- packages/kg-default-nodes/test/nodes/audio.test.ts
- packages/kg-lexical-html-renderer/test/render.test.ts
- packages/kg-default-nodes/src/utils/get-first-html-element.ts
- packages/kg-default-nodes/src/nodes/video/video-renderer.ts
- packages/kg-lexical-html-renderer/src/get-dynamic-data-nodes.ts
- packages/kg-default-nodes/src/nodes/product/product-renderer.ts
- packages/kg-default-nodes/src/nodes/paywall/paywall-renderer.ts
- packages/kg-lexical-html-renderer/src/transformers/element/paragraph.ts
- packages/kg-lexical-html-renderer/src/transformers/element/blockquote.ts
- packages/kg-default-nodes/src/nodes/audio/audio-renderer.ts
- packages/kg-default-nodes/src/nodes/html/html-renderer.ts
- packages/kg-default-nodes/src/nodes/email/email-renderer.ts
- packages/kg-default-nodes/src/nodes/signup/signup-renderer.ts
- packages/kg-default-nodes/src/nodes/markdown/markdown-renderer.ts
- packages/kg-default-nodes/src/nodes/call-to-action/calltoaction-renderer.ts
- packages/kg-lexical-html-renderer/src/transformers/element/aside.ts
- packages/kg-lexical-html-renderer/src/transformers/element/list.ts
- packages/kg-default-nodes/src/export-dom.ts
- packages/kg-lexical-html-renderer/src/transformers/element/heading.ts
- packages/kg-default-nodes/src/KoenigDecoratorNode.ts
- packages/kg-lexical-html-renderer/test/cards.test.ts
- packages/kg-default-nodes/src/utils/visibility.ts
Summary
kg-lexical-html-rendererfromlib/tosrc/and move its source, build, and tests to a TypeScript/ESM setupkg-default-nodescard export typing, Koenig card contracts, and visibility/render helpers with Lexical-compatible DOM export output and the renderer's TypeScript usage