fix: expose types #231
fix: expose types #231
Conversation
WalkthroughCentralizes public types in src/types.ts and re-exports via src/index.ts. Introduces a global zig namespace for ambient Zig typings and updates src/logic/index.d.ts to use zig.* types. Moves asset module declarations (*.wgsl, *.svg, *.ttf) from global.d.ts to modules.d.ts. Updates many files to import types explicitly. Adjusts tsconfig settings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant App as initCreator (TS)
participant Logic as Logic (zig bindings)
note over App,Logic: Signatures migrated to zig.* types
Caller->>App: initCreator({ onAssetSelect: (assetId: Id) => void })
App->>Logic: connectOnAssetSelectionCallback(cb: (data: zig.Id) => void)
Logic-->>App: emit selection (id: zig.Id)
App->>App: cast [...id] as Id
App-->>Caller: onAssetSelect(assetId: Id)
note right of Caller: Callback now typed with Id
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR exposes types by moving type definitions from the main index file to a dedicated types file and updating import statements across the codebase. The main purpose is to fix type exposure by centralizing type definitions and ensuring they're properly accessible.
- Moved all type definitions from
src/index.tstosrc/types.ts - Updated import statements across utility functions and modules to use the centralized types
- Added
modules.d.tsto the TypeScript build configuration for better type declaration generation
Reviewed Changes
Copilot reviewed 27 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.build.json | Added modules.d.ts to include array and fixed typo in comment |
| src/types.ts | Added comprehensive type definitions moved from index.ts |
| src/index.ts | Removed type definitions and added imports/exports from types file |
| src/utils/*.ts | Updated to import Point/PointUV types from centralized types file |
| src/svgToShapes/*.ts | Updated to import types from centralized types file |
| integration-tests/index.ts | Updated import to include Point type |
| ADRs/type of zig & exported types | Added documentation about typing strategy |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/normalizeVec2.ts (1)
3-9: Handle zero-length vector to prevent NaN.If
vecis{x: 0, y: 0},lengthwill be0, causing division by zero on lines 6-7. This produces{x: NaN, y: NaN}, which will corrupt downstream calculations.Apply this diff to handle the edge case:
export default function normalizeVec2(vec: Point): Point { const length = Math.sqrt(vec.x * vec.x + vec.y * vec.y) + if (length === 0) { + return { x: 0, y: 0 } + } return { x: vec.x / length, y: vec.y / length, } }
🧹 Nitpick comments (3)
src/svgToShapes/parsePathData.ts (1)
1-1: Switch to type-only import for Point.
Pointis only used as a type, so mark this as a type import to keep the compiled output clean and satisfyverbatimModuleSyntax/preserveValueImportsconfigurations. Please mirror the change wherever the new'types'entry point is pulled in purely for types.-import { Point } from 'types' +import type { Point } from 'types'src/types.ts (1)
5-142: Centralized type surface looks solid. Optional refinements below.
- Consider making Id a readonly tuple to prevent accidental mutation: type Id = readonly [number, number, number, number].
- Consider replacing enum CreatorTool with a union type or a const object + as const for tree-shaking friendliness, unless you depend on a runtime enum.
Not blockers.
src/index.ts (1)
14-23: Use type-only imports to avoid unnecessary runtime imports.These are type positions; switch to
import typefor cleaner emitted JS.-import { +import type { CreatorTool, Id, PointUV, SerializedInputAsset, SerializedOutputAsset, ShapeProps, ZigAsset, } from './types' export * from './types'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (31)
ADRs/type of zig & exported types(1 hunks)global.d.ts(1 hunks)integration-tests/index.ts(1 hunks)modules.d.ts(1 hunks)src/WebGPU/textureCache.ts(1 hunks)src/fonts.ts(1 hunks)src/index.ts(2 hunks)src/logic/index.d.ts(6 hunks)src/pointer.ts(1 hunks)src/run.ts(1 hunks)src/sanitizeFill.ts(1 hunks)src/svgToShapes/boundingBox.ts(1 hunks)src/svgToShapes/collectShapesData.ts(1 hunks)src/svgToShapes/definitions.ts(1 hunks)src/svgToShapes/getProps.ts(1 hunks)src/svgToShapes/parseEllipse.ts(1 hunks)src/svgToShapes/parsePathData.ts(1 hunks)src/svgToShapes/parseRect.ts(1 hunks)src/svgToShapes/radialGradient.ts(1 hunks)src/svgToShapes/utils.ts(1 hunks)src/types.ts(1 hunks)src/utils/distancePointToLine.ts(1 hunks)src/utils/getAngle.ts(1 hunks)src/utils/getBezierPos.ts(1 hunks)src/utils/getBezierTan.ts(1 hunks)src/utils/getCurveLength.ts(1 hunks)src/utils/getDefaultPoints.ts(1 hunks)src/utils/getDistance.ts(1 hunks)src/utils/normalizeVec2.ts(1 hunks)tsconfig.build.json(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/types.ts (2)
src/logic/index.d.ts (1)
Point(81-81)src/svgToShapes/boundingBox.ts (1)
BoundingBox(7-32)
global.d.ts (2)
src/logic/index.d.ts (1)
Point(81-81)src/types.ts (6)
Point(5-8)PointUV(10-15)BoundingBox(17-22)Id(25-25)ShapeProps(51-55)ZigAsset(142-142)
src/index.ts (1)
src/types.ts (1)
Id(25-25)
src/logic/index.d.ts (2)
src/types.ts (6)
PointUV(10-15)Point(5-8)ShapeProps(51-55)ZigAsset(142-142)Id(25-25)BoundingBox(17-22)src/svgToShapes/boundingBox.ts (1)
BoundingBox(7-32)
🔇 Additional comments (23)
src/svgToShapes/utils.ts (1)
3-3: LGTM!Type imports correctly added to support the centralized type surface.
src/svgToShapes/parseEllipse.ts (1)
1-2: LGTM!Type import correctly added for Point usage in function signature and curve definitions.
src/utils/getBezierTan.ts (1)
1-1: LGTM!Type import correctly added to support Point usage in function signature.
src/WebGPU/textureCache.ts (1)
6-6: LGTM!Type import correctly added for BoundingBox parameter annotation in startCache.
src/utils/getDefaultPoints.ts (1)
1-1: LGTM!Type import correctly added for PointUV return type annotation.
src/utils/getCurveLength.ts (1)
1-1: LGTM!Type import correctly added for Point parameter annotations in getCurveLength.
src/svgToShapes/getProps.ts (1)
7-7: LGTM!Type import correctly added for Color usage in gradient stop definitions.
src/run.ts (1)
26-26: LGTM!Type imports correctly added for BoundingBox and Point usage in cache callbacks and pick tracking.
integration-tests/index.ts (1)
1-1: LGTM!The explicit import of
Pointaligns with the centralized type surface and improves type safety.tsconfig.build.json (1)
10-10: LGTM!Adding
modules.d.tsto the include array ensures the new ambient module declarations for asset types are included in the build output.src/utils/getDistance.ts (1)
1-5: LGTM!The type annotations improve type safety without changing the function's behavior.
src/utils/getBezierPos.ts (1)
1-21: LGTM!The type annotations strengthen the function signature without altering its Bézier curve calculation logic.
ADRs/type of zig & exported types (1)
1-5: LGTM!The ADR clearly documents the rationale for using
declare globalblocks to expose types for Zig ambient definitions while maintaining a centralized type surface.src/utils/getAngle.ts (1)
1-5: LGTM!The type annotations improve type safety without changing the angle calculation logic.
global.d.ts (1)
3-15: Global zig namespace augmentation is fine; ensure inclusion order.Make sure global.d.ts is part of the TypeScript program (dev and build) so zig.* is available to src/logic/index.d.ts. If not already, add global.d.ts to the include/files list.
You can confirm with the same script shared in the modules.d.ts comment.
src/index.ts (2)
43-43: API change: onAssetSelect now expects Id (tuple) instead of a number.This is likely a breaking change for consumers. Please document it and provide a brief migration note in the changelog/README.
177-177: Good: clone and cast zig.Id to Id before emitting.The spread prevents accidental aliasing with Zig-managed memory and matches the Id tuple shape.
src/logic/index.d.ts (6)
23-37: Aligning addImage/addShape/resetAssets to zig. types looks correct.*The shift to zig.Point/PointUV/ShapeProps/ZigAsset is consistent with global augmentation.
Please ensure global.d.ts is included in the program so zig.* resolves for consumers.
38-39: Switched selection/update IDs to zig.Id — good.Matches the new public Id tuple.
81-84: Constructors returning zig. types are consistent.*Point and PtrI32 signatures align with the new namespaces.
127-129: Update callbacks now emit zig.ZigAsset[] and zig.Id — consistent.Matches the rest of the API.
135-139: Cache/select updates use zig.BoundingBox and zig.PointUV/ShapeProps — consistent.No issues.
160-167: Setters updated to zig. types — consistent.*setSelectedAssetProps and setSelectedAssetBounds signatures look good.
|
🎉 This PR is included in version 0.1.0-next.27 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Refactor
Chores