Skip to content

fix(RuleY): widen data generic to accept raw values #572

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
ljodea wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(RuleY): widen data generic to accept raw values #572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0514b51
test(svelteplot): add scoped check baseline for RuleY contract
ljodea Jun 26, 2026
ac0be9e
test(svelteplot): add RuleY source contract tests (RED)
ljodea Jun 26, 2026
d280837
fix(RuleY): widen Datum generic and remove DataRow cast
ljodea Jun 26, 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: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"type": "module",
"scripts": {
"build": "vite build",
"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json && pnpm --filter svelteplot check",
"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
"dev": "vite dev",
"docs": "npm run build && cd build && rsync --recursive . vis4.net:svelteplot/alpha0/",
Expand Down
1 change: 1 addition & 0 deletions packages/svelteplot/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
],
"scripts": {
"prepack": "npx svelte-package --input src && node scripts/fix-js-imports.js dist",
"check": "svelte-check --tsconfig tsconfig.check.json --threshold error",
"test": "vitest run",
"test:watch": "vitest"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/svelteplot/src/marks/RuleX.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<!-- @component
Renders vertical rule lines at specified x positions with customizable vertical range
-->
<script lang="ts" generics="Datum = DataRecord | RawValue">
<script lang="ts" generics="Datum extends DataRecord | RawValue = DataRecord | RawValue">
interface RuleXMarkProps extends Omit<BaseMarkProps<Datum>, 'fill' | 'fillOpacity'> {
/** the input data array; each element becomes one vertical rule */
data?: Datum[];
Expand Down
10 changes: 4 additions & 6 deletions packages/svelteplot/src/marks/RuleY.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<!-- @component
Renders horizontal rule lines at specified y positions with customizable horizontal range
-->
<script lang="ts" generics="Datum = DataRecord">
<script lang="ts" generics="Datum extends DataRecord | RawValue = DataRecord | RawValue">

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve raw datums for explicit y accessors

With this widened generic, calls such as <RuleY data={[5]} y={(d) => d} /> now type-check with d as the raw number. However recordizeY only creates RAW_VALUE wrappers when y == null; when an explicit y callback is present it indexes the primitive as an object, so the callback receives the wrapper record instead of 5 and produces an invalid channel value. Either preserve raw datums even when explicit channels are supplied or keep the raw-value contract limited to the no-y shorthand.

Useful? React with 👍 / 👎.

@ljodea ljodea Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fair catch on recordizeY behavior, but out of scope for this PR.

The raw shorthand this fixes is the documented no-y form (<RuleY data={[50, 100, 150]} />). Combining a raw value array with an explicit y accessor was already handled the same way on main (RuleY still cast to DataRow[] before recordizeY); this diff only changes the declared generic and removes that cast.

recordizeY only wraps primitives when y == null by design — that is shared transform logic, not something RuleY's generic widening introduced. Tightening or fixing that path (raw data + explicit channel) belongs in a follow-up on recordizeY/recordizeX, not in the #41 type-honesty fix.

interface RuleYMarkProps extends Omit<BaseMarkProps<Datum>, 'fill' | 'fillOpacity'> {
/** the input data array; each element becomes one horizontal rule */
data?: Datum[];
Expand All @@ -27,10 +27,10 @@
import { resolveProp, resolveStyles } from '../helpers/resolve.js';
import type {
DataRecord,
DataRow,
BaseMarkProps,
ConstantAccessor,
ChannelAccessor
ChannelAccessor,
RawValue
} from '../types/index.js';
import { getPlotDefaults } from '../hooks/plotDefaults.js';
import { IS_SORTED } from 'svelteplot/transforms/sort';
Expand All @@ -52,9 +52,7 @@
});

const plot = usePlot();
const args = $derived(
recordizeY({ data: data as DataRow[], ...options }, { withIndex: false })
);
const args = $derived(recordizeY({ data, ...options }, { withIndex: false }));
</script>

<Mark type="ruleY" channels={['y', 'x1', 'x2', 'stroke', 'opacity', 'strokeOpacity']} {...args}>
Expand Down
2 changes: 2 additions & 0 deletions packages/svelteplot/tests/check-ambient.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
declare module 'interval-tree-1d';
declare module 'd3-time';
18 changes: 18 additions & 0 deletions packages/svelteplot/tests/ruleY.contract.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { describe, it, expect } from 'vitest';
import { readFileSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import path from 'node:path';

const ruleYPath = path.join(path.dirname(fileURLToPath(import.meta.url)), '../src/marks/RuleY.svelte');

describe('RuleY public type contract', () => {
const src = readFileSync(ruleYPath, 'utf8');

it('declares Datum extends DataRecord | RawValue', () => {
expect(src).toMatch(/generics="Datum extends DataRecord \| RawValue/);
});

it('passes data to recordizeY without a DataRow cast', () => {
expect(src).not.toMatch(/data as DataRow\[\]/);
});
});
12 changes: 12 additions & 0 deletions packages/svelteplot/tests/ruleY.contract.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import type { ComponentProps } from 'svelte';
import RuleY from '../src/marks/RuleY.svelte';

type AssertRuleYProps<T extends ComponentProps<typeof RuleY>> = T;

// Regression guards (green on main; stay green after fix)
type _RawBaseline = AssertRuleYProps<{ data: [0, 1] }>;
type _RecordRows = AssertRuleYProps<{ data: { y: number }[]; y: 'y' }>;
type AssertTrue<T extends true> = T;
type RuleYData = NonNullable<ComponentProps<typeof RuleY>['data']>;
type _NumberArrayAssignable = [number[]] extends [RuleYData] ? true : false;
type _ExpectNumberArray = AssertTrue<_NumberArrayAssignable>;
23 changes: 23 additions & 0 deletions packages/svelteplot/tsconfig.check.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"paths": {
"svelteplot": ["./src/index.ts"],
"svelteplot/*": ["./src/*"]
}
},
"include": [
"tests/ruleY.contract.ts",
"tests/check-ambient.d.ts",
"src/**/*.ts",
"src/**/*.svelte",
"src/**/*.js"
],
"exclude": [
"tests/**/*.test.ts",
"tests/**/*.test.svelte.ts",
"tests/**/*.svelte",
"src/**/*.test.ts",
"src/**/*.spec.ts"
]
}
Loading