Skip to content

Migrate kg-card-factory to TypeScript #1758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kevinansfield wants to merge 2 commits into
base: main
Choose a base branch
from
+318 −221
Open

Migrate kg-card-factory to TypeScript #1758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a22794d
Rename files for kg-card-factory TypeScript migration
kevinansfield Mar 16, 2026
559817e
Migrate kg-card-factory to TypeScript
kevinansfield Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/kg-card-factory/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
build/
tsconfig.tsbuildinfo
47 changes: 19 additions & 28 deletions packages/kg-card-factory/eslint.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,33 @@
import {fixupPluginRules} from '@eslint/compat';
import {defineConfig} from 'eslint/config';
import eslint from '@eslint/js';
import ghostPlugin from 'eslint-plugin-ghost';
import globals from 'globals';
import tseslint from 'typescript-eslint';

const ghost = fixupPluginRules(ghostPlugin);

export default [
export default defineConfig([
{ignores: ['build/**']},
eslint.configs.recommended,
{
files: ['**/*.js'],
plugins: {ghost},
files: ['**/*.ts'],
extends: [
eslint.configs.recommended,
tseslint.configs.recommended
],
languageOptions: {
globals: globals.node
parserOptions: {ecmaVersion: 2022, sourceType: 'module'}
},
plugins: {ghost: ghostPlugin},
rules: {
...ghostPlugin.configs.node.rules,
// match ESLint 8 behavior for catch clause variables
'no-unused-vars': ['error', {caughtErrors: 'none'}],
// disable rules incompatible with ESLint 9 flat config
'ghost/filenames/match-exported-class': 'off',
'ghost/filenames/match-exported': 'off',
'ghost/filenames/match-regex': 'off'
...ghostPlugin.configs.ts.rules,
'@typescript-eslint/no-explicit-any': 'error'
}
},
{
files: ['test/**/*.js'],
plugins: {ghost},
languageOptions: {
globals: {
...globals.node,
...globals.mocha,
should: true,
sinon: true
}
},
files: ['test/**/*.ts'],
rules: {
...ghostPlugin.configs.test.rules
...ghostPlugin.configs['ts-test'].rules,
'ghost/mocha/no-global-tests': 'off',
'ghost/mocha/handle-done-callback': 'off',
'ghost/mocha/no-mocha-arrows': 'off',
'ghost/mocha/max-top-level-suites': 'off'
}
}
];
]);
1 change: 0 additions & 1 deletion packages/kg-card-factory/index.js
View file
Open in desktop

This file was deleted.

86 changes: 0 additions & 86 deletions packages/kg-card-factory/lib/CardFactory.js
View file
Open in desktop

This file was deleted.

31 changes: 25 additions & 6 deletions packages/kg-card-factory/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,50 @@
{
"name": "@tryghost/kg-card-factory",
"version": "5.1.12",
"repository": "https://github.com/TryGhost/Koenig/tree/master/packages/kg-card-factory",
"repository": "https://github.com/TryGhost/Koenig/tree/main/packages/kg-card-factory",
"author": "Ghost Foundation",
"license": "MIT",
"main": "index.js",
"main": "build/cjs/index.js",
"module": "build/esm/index.js",
"types": "build/esm/index.d.ts",
"exports": {
".": {
"types": "./build/esm/index.d.ts",
"import": "./build/esm/index.js",
"require": "./build/cjs/index.js"
}
},
"scripts": {
"dev": "echo \"Implement me!\"",
"test": "NODE_ENV=testing c8 --all --reporter text --reporter cobertura mocha './test/**/*.test.js'",
"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",
"test": "NODE_ENV=testing c8 --all --reporter text --reporter cobertura mocha --require tsx './test/**/*.test.ts'",
"lint": "eslint . --cache",
"posttest": "yarn lint"
},
"engines": {
"node": "^22.13.1 || ^24.0.0"
},
"files": [
"index.js",
"lib"
"build"
],
"publishConfig": {
"access": "public"
},
"devDependencies": {
"@eslint/js": "9.39.4",
"@types/mocha": "10.0.10",
"@types/node": "22.15.29",
"@types/should": "13.0.0",
"@types/sinon": "17.0.4",
"c8": "11.0.0",
"mocha": "11.7.5",
"should": "13.2.3",
"simple-dom": "1.4.0",
"sinon": "21.0.2"
"sinon": "21.0.2",
"tsx": "4.19.4",
"typescript": "5.9.3",
"typescript-eslint": "8.33.1"
}
}
119 changes: 119 additions & 0 deletions packages/kg-card-factory/src/CardFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
export interface FactoryOptions {
siteUrl?: string;
[key: string]: unknown;
}

export interface CardPayload {
[key: string]: unknown;
}

export interface CardTransformOptions {
assetsOnly?: boolean;
siteUrl?: string;
[key: string]: unknown;
}

export interface DomNode {
nodeType?: number;
nodeValue?: string;
appendChild?(child: unknown): void;
[key: string]: unknown;
}

export interface DomProvider {
createComment(text: string): DomNode;
createDocumentFragment(): DomNode;
createElement?(tag: string): DomNode;
createTextNode?(text: string): DomNode;
[key: string]: unknown;
}

export interface CardRenderEnv {
dom: DomProvider;
[key: string]: unknown;
}

export interface CardRenderArgs {
env: CardRenderEnv;
payload: CardPayload;
options?: Record<string, unknown>;
}

export interface CardDefinition {
name: string;
type: string;
config?: { commentWrapper?: boolean };
render(args: CardRenderArgs): DomNode;
absoluteToRelative?(payload: CardPayload, options: CardTransformOptions): CardPayload;
relativeToAbsolute?(payload: CardPayload, options: CardTransformOptions): CardPayload;
toTransformReady?(payload: CardPayload, options: CardTransformOptions): CardPayload;
}

export default class CardFactory {
factoryOptions: FactoryOptions;

constructor(options: FactoryOptions = {}) {
this.factoryOptions = options;
}

createCard(card: CardDefinition) {
const {factoryOptions} = this;
const {name, type, config = {}} = card;

return {
name,
type,
factoryOptions,

render({env, payload, options}: CardRenderArgs): DomNode {
const {dom} = env;
const cardOptions = Object.assign({}, factoryOptions, options);
const cardOutput = card.render({env, payload, options: cardOptions});

if (cardOutput.nodeType === 3 && cardOutput.nodeValue === '') {
return cardOutput;
}

if (config.commentWrapper) {
const cleanName = name.replace(/^card-/, '');
const beginComment = dom.createComment(`kg-card-begin: ${cleanName}`);
const endComment = dom.createComment(`kg-card-end: ${cleanName}`);
const fragment = dom.createDocumentFragment();
fragment.appendChild!(beginComment);
fragment.appendChild!(cardOutput);
fragment.appendChild!(endComment);
return fragment;
}

return cardOutput;
},

absoluteToRelative(payload: CardPayload, _options?: CardTransformOptions) {
if (card.absoluteToRelative) {
const defaultOptions = {assetsOnly: false, siteUrl: factoryOptions.siteUrl};
const options = Object.assign({}, defaultOptions, _options);
return card.absoluteToRelative(payload, options);
}
return payload;
},

relativeToAbsolute(payload: CardPayload, _options?: CardTransformOptions) {
if (card.relativeToAbsolute) {
const defaultOptions = {assetsOnly: false, siteUrl: factoryOptions.siteUrl};
const options = Object.assign({}, defaultOptions, _options);
return card.relativeToAbsolute(payload, options);
}
return payload;
},

toTransformReady(payload: CardPayload, _options?: CardTransformOptions) {
if (card.toTransformReady) {
const defaultOptions = {assetsOnly: false, siteUrl: factoryOptions.siteUrl};
const options = Object.assign({}, defaultOptions, _options);
return card.toTransformReady(payload, options);
}
return payload;
}
};
}
}
2 changes: 2 additions & 0 deletions packages/kg-card-factory/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export {default} from './CardFactory.js';
Copy link

@coderabbitai coderabbitai bot Mar 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In TypeScript, when compilerOptions.moduleisCommonJS, how does export { default } from "./CardFactory.js"compile, and doesrequire("./build/index.js")return the default export directly or an object with a.default property?

💡 Result:

With compilerOptions.module: "CommonJS", TypeScript keeps the “default export” as a property on exports, and a re-export like:

export { default } from "./CardFactory.js";

compiles to CommonJS roughly like:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });

var CardFactory_js_1 = require("./CardFactory.js");
Object.defineProperty(exports, "default", {
  enumerable: true,
  get: function () { return CardFactory_js_1.default; }
});

So:

  • require("./build/index.js") returns the exports object, not the default value directly.
  • The re-exported default is at require("./build/index.js").default.
    This is the standard interop shape TypeScript emits for export default when targeting CommonJS. [1][2][3]

Sources:
[1] https://www.typescriptlang.org/docs/handbook/modules/appendices/esm-cjs-interop.html
[2] https://blog.andrewbran.ch/default-exports-in-commonjs-libraries/
[3] https://stackoverflow.com/questions/60577733/not-able-to-require-default-exported-function-from-typescript-file-in-nodejs

🏁 Script executed:

# Check if the kg-card-factory package exists and inspect its configuration
if [ -d "packages/kg-card-factory" ]; then
  echo "=== Package exists ==="
  echo "=== tsconfig.json ==="
  cat packages/kg-card-factory/tsconfig.json 2>/dev/null || echo "File not found"
  echo ""
  echo "=== package.json (main field) ==="
  cat packages/kg-card-factory/package.json 2>/dev/null | grep -A 5 -B 5 '"main"' || echo "Not found"
  echo ""
  echo "=== index.ts content ==="
  cat packages/kg-card-factory/src/index.ts 2>/dev/null || echo "File not found"
else
  echo "Package directory does not exist"
fi

Repository: TryGhost/Koenig

Length of output: 1030

🏁 Script executed:

# Search for imports of kg-card-factory in the codebase to identify consumers
echo "=== Searching for imports of kg-card-factory ==="
rg "from ['\"]@tryghost/kg-card-factory|require\(['\"]@tryghost/kg-card-factory" --type ts --type js -n

Repository: TryGhost/Koenig

Length of output: 107

This changes the CommonJS entry shape.

With compilerOptions.module set to CommonJS, the re-export export {default} from './CardFactory.js' compiles to exports.default = ..., so require('@tryghost/kg-card-factory') returns an object with a .default property instead of returning CardFactory directly. This is a breaking change for existing CommonJS consumers unless the package is intentionally switching to true ESM.

Possible compatibility-preserving fix 
-export {default} from './CardFactory.js';
+import CardFactory from './CardFactory.js';
+
+export = CardFactory;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export {default} from './CardFactory.js';
import CardFactory from './CardFactory.js';
export = CardFactory;
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/kg-card-factory/src/index.ts` at line 1, The current re-export
`export {default} from './CardFactory.js'` changes the CommonJS shape (it
becomes `exports.default = ...`) so require('@tryghost/kg-card-factory') returns
an object with .default; replace the ES-style re-export with a
CommonJS-compatible export so requiring the package returns CardFactory
directly—for example, in packages/kg-card-factory/src/index.ts import or require
'./CardFactory.js' and assign the export to module.exports (e.g., const
CardFactory = require('./CardFactory.js'); module.exports = CardFactory;),
ensuring consumers using require() get the CardFactory value rather than an
object with a .default property.

✅ Addressed in commits 0f13173 to 89707bf

Copy link
Member Author

@kevinansfield kevinansfield Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

esModuleInterop handles this for TS consumers, and the existing JS code already used export default so isn't this the same behaviour as before?

Copy link

@coderabbitai coderabbitai bot Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevinansfield, you raise a fair point — esModuleInterop does handle this correctly for TypeScript consumers, allowing import CardFactory from '@tryghost/kg-card-factory' to work seamlessly even when the CJS output exposes exports.default.

However, there's a subtle distinction worth noting: the old index.js actually used module.exports = require('./lib/CardFactory') (not export default), so the old CJS shape returned CardFactory directly from require(...), while the new CJS build of export {default} would produce exports.default = CardFactory. That said, if all consumers in the monorepo are TypeScript projects with esModuleInterop: true (which the search results suggest is the case), this shape difference has no practical impact — your existing consumers would be unaffected.

So my original comment was overstated for your specific use case. You're right that for TypeScript consumers, this is effectively equivalent behavior.

✏️ Learnings added 
Learnt from: kevinansfield
Repo: TryGhost/Koenig PR: 1758
File: packages/kg-card-factory/src/index.ts:1-1
Timestamp: 2026-03-17T22:14:21.295Z
Learning: In the TryGhost/Koenig monorepo, all consumers of `tryghost/kg-card-factory` are TypeScript packages with `esModuleInterop: true`. The package uses ESM-style `export {default}` in `src/index.ts`, which is fine for these consumers even when compiled to CJS. Raw CommonJS `require()` callers are not a concern for this package.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

export type {FactoryOptions, CardPayload, CardTransformOptions, CardRenderEnv, CardRenderArgs, CardDefinition} from './CardFactory.js';
Loading
Loading