Skip to content

Commit 1f53742

Browse files
committed
fix: address ink-compat PR review feedback and CI regressions
1 parent 63dfb9d commit 1f53742

27 files changed

Lines changed: 667 additions & 160 deletions

File tree

biome.json

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,25 @@
2727
"performance": {
2828
"noDelete": "off"
2929
},
30+
"suspicious": {
31+
"noControlCharactersInRegex": "off"
32+
}
33+
}
34+
}
35+
},
36+
{
37+
"include": [
38+
"packages/ink-compat/src/runtime/bridge.ts",
39+
"packages/ink-compat/src/runtime/render.ts",
40+
"packages/ink-compat/src/shims/ink-gradient.tsx",
41+
"packages/ink-compat/src/shims/ink-spinner.tsx",
42+
"packages/ink-compat/src/translation/colorMap.ts",
43+
"packages/ink-compat/src/translation/propsToVNode.ts"
44+
],
45+
"linter": {
46+
"rules": {
3047
"style": {
3148
"noNonNullAssertion": "off"
32-
},
33-
"suspicious": {
34-
"noControlCharactersInRegex": "off",
35-
"noExplicitAny": "off"
3649
}
3750
}
3851
}
@@ -44,6 +57,12 @@
4457
"correctness": {
4558
"noChildrenProp": "off",
4659
"useExhaustiveDependencies": "off"
60+
},
61+
"style": {
62+
"noNonNullAssertion": "off"
63+
},
64+
"suspicious": {
65+
"noExplicitAny": "off"
4766
}
4867
}
4968
}

docs/architecture/ink-compat.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
## Package topology
1818

19-
```
19+
```text
2020
packages/ink-compat/
2121
src/
2222
components/ // Ink-compatible component wrappers

packages/core/src/renderer/renderToDrawlist/boxBorder.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,14 +126,22 @@ function renderBorderFrame(
126126
// Top edge (fallback to horizontal cap when missing a vertical side).
127127
const leftCap = sides.left ? glyphs.TL : glyphs.H;
128128
const rightCap = sides.right ? glyphs.TR : glyphs.H;
129-
builder.drawText(x0, y0, `${leftCap}${glyphs.H.repeat(innerW)}${rightCap}`, style);
129+
if (rect.w <= 1) {
130+
builder.drawText(x0, y0, glyphs.H, style);
131+
} else {
132+
builder.drawText(x0, y0, `${leftCap}${glyphs.H.repeat(innerW)}${rightCap}`, style);
133+
}
130134
}
131135

132136
if (sides.bottom) {
133137
// Bottom edge (fallback to horizontal cap when missing a vertical side).
134138
const leftCap = sides.left ? glyphs.BL : glyphs.H;
135139
const rightCap = sides.right ? glyphs.BR : glyphs.H;
136-
builder.drawText(x0, y1, `${leftCap}${glyphs.H.repeat(innerW)}${rightCap}`, style);
140+
if (rect.w <= 1) {
141+
builder.drawText(x0, y1, glyphs.H, style);
142+
} else {
143+
builder.drawText(x0, y1, `${leftCap}${glyphs.H.repeat(innerW)}${rightCap}`, style);
144+
}
137145
}
138146
} else {
139147
if (sides.top) {

packages/core/src/renderer/renderToDrawlist/widgets/renderTextWidgets.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ export function renderTextWidgets(
349349
builder.popClip();
350350
}
351351

352-
if (cursorInfo && cursorMeta.focused) {
352+
if (!transform && cursorInfo && cursorMeta.focused) {
353353
let remaining = cursorOffset;
354354
let cursorLine = 0;
355355
for (let i = 0; i < visibleCount; i++) {
@@ -374,10 +374,6 @@ export function renderTextWidgets(
374374
}
375375

376376
const transformedText = transformLine(text, 0);
377-
const cursorX = Math.min(
378-
overflowW,
379-
measureTextCells(transformedText.slice(0, Math.min(cursorOffset, transformedText.length))),
380-
);
381377

382378
// Avoid measuring in the common ASCII case.
383379
const fits =
@@ -386,7 +382,13 @@ export function renderTextWidgets(
386382

387383
if (fits) {
388384
builder.drawText(rect.x, rect.y, transformedText, style);
389-
if (cursorInfo && cursorMeta.focused) {
385+
if (!transform && cursorInfo && cursorMeta.focused) {
386+
const cursorX = Math.min(
387+
overflowW,
388+
measureTextCells(
389+
transformedText.slice(0, Math.min(cursorOffset, transformedText.length)),
390+
),
391+
);
390392
resolvedCursor = {
391393
x: rect.x + cursorX,
392394
y: rect.y,
@@ -421,7 +423,13 @@ export function renderTextWidgets(
421423
} else {
422424
builder.drawText(rect.x, rect.y, displayText, style);
423425
}
424-
if (cursorInfo && cursorMeta.focused) {
426+
if (!transform && cursorInfo && cursorMeta.focused) {
427+
const cursorX = Math.min(
428+
overflowW,
429+
measureTextCells(
430+
transformedText.slice(0, Math.min(cursorOffset, transformedText.length)),
431+
),
432+
);
425433
resolvedCursor = {
426434
x: rect.x + cursorX,
427435
y: rect.y,

packages/core/src/testing/renderer.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,7 @@ export function createTestRenderer(opts: TestRendererOptions = {}): TestRenderer
452452
const defaultTick = opts.tick ?? 0;
453453
const trace = opts.trace;
454454
const defaultTraceDetail = opts.traceDetail === true;
455+
let warnedTraceDetailWithoutTrace = false;
455456
let renderId = 0;
456457

457458
const render = (vnode: VNode, renderOpts: TestRenderOptions = {}): TestRenderResult => {
@@ -463,6 +464,12 @@ export function createTestRenderer(opts: TestRendererOptions = {}): TestRenderer
463464
const tick = renderOpts.tick ?? defaultTick;
464465
const theme = renderOpts.theme ?? rendererTheme;
465466
const traceDetail = renderOpts.traceDetail ?? defaultTraceDetail;
467+
if (traceDetail && !trace && !warnedTraceDetailWithoutTrace) {
468+
warnedTraceDetailWithoutTrace = true;
469+
console.warn(
470+
"[core/testing] createTestRenderer: traceDetail=true has no effect without a trace callback.",
471+
);
472+
}
466473

467474
const commitStartedAt = Date.now();
468475
const committed = commitVNodeTree(prevRoot, vnode, { allocator });

packages/ink-compat/src/__tests__/apps/counter.test.tsx

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,51 +46,56 @@ const Counter: React.FC = () => {
4646
);
4747
};
4848

49+
function readCount(frame: string): number {
50+
const match = frame.match(/Count:\s*(\d+)/);
51+
assert.ok(match, "count line should exist");
52+
return Number.parseInt(match[1]!, 10);
53+
}
54+
4955
test("counter: renders initial state", () => {
5056
const { lastFrame } = render(React.createElement(Counter));
5157
const frame = lastFrame();
5258
assert.ok(frame.includes("Counter App"), "should show title");
5359
assert.ok(frame.includes("Count:"), "should show count label");
54-
assert.ok(frame.includes("0"), "should show initial count 0");
60+
assert.equal(readCount(frame), 0, "should show initial count 0");
5561
});
5662

5763
test("counter: up arrow increments", () => {
5864
const { lastFrame, stdin } = render(React.createElement(Counter));
5965
stdin.write("\u001b[A"); // up arrow
60-
const frame = lastFrame();
61-
assert.ok(frame.includes("1"), "count should be 1 after up arrow");
66+
assert.equal(readCount(lastFrame()), 1, "count should be 1 after up arrow");
6267
});
6368

6469
test("counter: k key increments", () => {
6570
const { lastFrame, stdin } = render(React.createElement(Counter));
6671
stdin.write("k");
67-
assert.ok(lastFrame().includes("1"), "count should be 1 after k");
72+
assert.equal(readCount(lastFrame()), 1, "count should be 1 after k");
6873
stdin.write("k");
69-
assert.ok(lastFrame().includes("2"), "count should be 2 after second k");
74+
assert.equal(readCount(lastFrame()), 2, "count should be 2 after second k");
7075
});
7176

7277
test("counter: down arrow decrements", () => {
7378
const { lastFrame, stdin } = render(React.createElement(Counter));
7479
stdin.write("k"); // go to 1
7580
stdin.write("k"); // go to 2
7681
stdin.write("\u001b[B"); // down arrow → 1
77-
assert.ok(lastFrame().includes("1"), "count should be 1 after decrement");
82+
assert.equal(readCount(lastFrame()), 1, "count should be 1 after decrement");
7883
});
7984

8085
test("counter: does not go below zero", () => {
8186
const { lastFrame, stdin } = render(React.createElement(Counter));
8287
stdin.write("\u001b[B"); // down arrow at 0
83-
assert.ok(lastFrame().includes("0"), "count should stay at 0");
88+
assert.equal(readCount(lastFrame()), 0, "count should stay at 0");
8489
});
8590

8691
test("counter: r resets to zero", () => {
8792
const { lastFrame, stdin } = render(React.createElement(Counter));
8893
stdin.write("k");
8994
stdin.write("k");
9095
stdin.write("k");
91-
assert.ok(lastFrame().includes("3"), "count should be 3");
96+
assert.equal(readCount(lastFrame()), 3, "count should be 3");
9297
stdin.write("r");
93-
assert.ok(lastFrame().includes("0"), "count should reset to 0");
98+
assert.equal(readCount(lastFrame()), 0, "count should reset to 0");
9499
});
95100

96101
test("counter: renders border", () => {

packages/ink-compat/src/__tests__/integration/basic.test.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,11 +199,11 @@ test("newline renders multi-line text", () => {
199199
});
200200

201201
test("useInput handles simulated key press", () => {
202-
const seen: Array<{ input: string; up: boolean; ctrl: boolean }> = [];
202+
const seen: Array<{ input: string; up: boolean; ctrl: boolean; escape: boolean }> = [];
203203

204204
function App(): React.ReactElement {
205205
useInput((input, key) => {
206-
seen.push({ input, up: key.upArrow, ctrl: key.ctrl });
206+
seen.push({ input, up: key.upArrow, ctrl: key.ctrl, escape: key.escape });
207207
});
208208
return React.createElement(Text, null, "Input");
209209
}
@@ -212,10 +212,12 @@ test("useInput handles simulated key press", () => {
212212
result.stdin.write("\u001b[A");
213213
result.stdin.write("q");
214214
result.stdin.write("\u0001");
215+
result.stdin.write("\u001b");
215216

216-
assert.deepEqual(seen[0], { input: "", up: true, ctrl: false });
217-
assert.deepEqual(seen[1], { input: "q", up: false, ctrl: false });
218-
assert.deepEqual(seen[2], { input: "a", up: false, ctrl: true });
217+
assert.deepEqual(seen[0], { input: "", up: true, ctrl: false, escape: false });
218+
assert.deepEqual(seen[1], { input: "q", up: false, ctrl: false, escape: false });
219+
assert.deepEqual(seen[2], { input: "a", up: false, ctrl: true, escape: false });
220+
assert.deepEqual(seen[3], { input: "", up: false, ctrl: false, escape: true });
219221
});
220222

221223
test("useInput parses kitty keyboard CSI-u sequences", async () => {

packages/ink-compat/src/__tests__/reconciler/hostConfig.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,11 @@ test("commit update updates node props and text", () => {
7575
const node = rootNode.children[0]!;
7676

7777
commitSync(root, React.createElement("ink-text", { color: "blue" }, "New"));
78+
const nodeAfter = rootNode.children[0]!;
7879

79-
assert.equal(node.props["color"], "blue");
80-
assert.equal(node.children[0]?.textContent, "New");
80+
assert.strictEqual(node, nodeAfter, "should be same instance (in-place update)");
81+
assert.equal(nodeAfter.props["color"], "blue");
82+
assert.equal(nodeAfter.children[0]?.textContent, "New");
8183
});
8284

8385
test("prepareUpdate performs shallow comparison without children/ref", () => {

packages/ink-compat/src/__tests__/reconciler/types.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,44 @@ test("insertBefore appends when target is missing", () => {
7171

7272
assert.deepEqual(parent.children, [childA, childB]);
7373
});
74+
75+
test("appendChild moves existing child without duplication", () => {
76+
const parent = createHostNode("ink-box", {});
77+
const childA = createHostNode("ink-text", { id: "a" });
78+
const childB = createHostNode("ink-text", { id: "b" });
79+
80+
appendChild(parent, childA);
81+
appendChild(parent, childB);
82+
appendChild(parent, childA);
83+
84+
assert.deepEqual(parent.children, [childB, childA]);
85+
assert.equal(parent.children.filter((child) => child === childA).length, 1);
86+
});
87+
88+
test("insertBefore moves existing child without duplication", () => {
89+
const parent = createHostNode("ink-box", {});
90+
const childA = createHostNode("ink-text", { id: "a" });
91+
const childB = createHostNode("ink-text", { id: "b" });
92+
const childC = createHostNode("ink-text", { id: "c" });
93+
94+
appendChild(parent, childA);
95+
appendChild(parent, childB);
96+
appendChild(parent, childC);
97+
insertBefore(parent, childC, childA);
98+
99+
assert.deepEqual(parent.children, [childC, childA, childB]);
100+
assert.equal(parent.children.filter((child) => child === childC).length, 1);
101+
});
102+
103+
test("appendChild detaches from previous non-container parent", () => {
104+
const parentA = createHostNode("ink-box", { id: "a" });
105+
const parentB = createHostNode("ink-box", { id: "b" });
106+
const child = createHostNode("ink-text", { id: "x" });
107+
108+
appendChild(parentA, child);
109+
appendChild(parentB, child);
110+
111+
assert.deepEqual(parentA.children, []);
112+
assert.deepEqual(parentB.children, [child]);
113+
assert.equal(child.parent, parentB);
114+
});

packages/ink-compat/src/__tests__/runtime/newApis.test.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ test("export surface includes kitty flags/modifiers and useCursor", () => {
2626
assert.equal(kittyModifiers.ctrl, 4);
2727
});
2828

29-
test("useIsScreenReaderEnabled reads context flag", () => {
29+
test("useIsScreenReaderEnabled reads context flag", async () => {
3030
const rootNode = createHostContainer();
3131
const root = reconciler.createContainer(
3232
rootNode,
@@ -80,12 +80,18 @@ test("useIsScreenReaderEnabled reads context flag", () => {
8080
reconciler.flushSyncWork?.();
8181
reconciler.flushPassiveEffects?.();
8282
} else {
83-
reconciler.updateContainer(
84-
React.createElement(InkContext.Provider, { value: mockContext }, React.createElement(Probe)),
85-
root,
86-
null,
87-
() => {},
88-
);
83+
await new Promise<void>((resolve) => {
84+
reconciler.updateContainer(
85+
React.createElement(
86+
InkContext.Provider,
87+
{ value: mockContext },
88+
React.createElement(Probe),
89+
),
90+
root,
91+
null,
92+
() => resolve(),
93+
);
94+
});
8995
}
9096

9197
assert.equal(seen, true);

0 commit comments

Comments
 (0)