fix(RuleY): widen data generic to accept raw values#572
Conversation
Add tsconfig.check.json with svelteplot path aliases and ambient stubs so package-level svelte-check can gate type regressions without the 300+ errors from the full test suite. Wire into root pnpm check.
Assert RuleY.svelte declares the widened Datum generic and passes data to recordizeY without a DataRow cast. Fails on main until the fix lands.
Declare Datum extends DataRecord | RawValue so the public API matches what recordizeY accepts; pass data directly without as DataRow[]. Align RuleX generic constraint so Box.svelte's dynamic RuleMark union type-checks under the new scoped svelte-check gate. Closes #41
✅ Deploy Preview for svelteplot ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d280837b22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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"> |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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.
1 participant
Closes #41
RuleYdeclaredDatum = DataRecordbut docs/runtime use<RuleY data={[0]} />and the component castdata as DataRow[]beforerecordizeY. The public generic didn't match what recordize marks accept.Datum extends DataRecord | RawValue, passdatadirectly (no cast)svelte-check+ source contract tests so this stays honestRuleXgeneric alignment soBox.svelte's dynamicRuleMarkunion still type-checksPR #48 only changed prop types on marks; it didn't fix the generic boundary.