refactor: tutorial.js and sequential.js to typescript #665
Conversation
WalkthroughAdds TypeScript typings and null-safety to simulator tutorial code and one sequential helper: explicit parameter and return types, new tutorial interfaces, and additional guards. No control-flow or runtime behavior changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/simulator/src/sequential.ts(1 hunks)src/simulator/src/tutorials.ts(2 hunks)src/simulator/src/types/tutorials.types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/simulator/src/tutorials.ts (1)
src/simulator/src/types/tutorials.types.ts (2)
TourStep(9-13)DriverInstance(15-24)
🔇 Additional comments (4)
src/simulator/src/sequential.ts (1)
4-11: LGTM! Clean TypeScript conversion.The type annotations are appropriate, and the JSDoc improvements enhance documentation clarity.
src/simulator/src/tutorials.ts (3)
2-4: LGTM! Proper type imports and annotations.The import statement and
tourtype annotation correctly leverage the new type definitions.
92-133: Good defensive programming with null checks.The null guards and event typing improve type safety. The double cast pattern (
as unknown as DriverInstance) is acceptable for this TypeScript migration, though it indicates the customDriverInstanceinterface may not perfectly align with driver.js's native types.
142-150: No behavior change detected—original concern invalid.The change from storing boolean
trueto string'true'does not affect functionality. Insrc/simulator/src/setup.js(line 157), the value is read using a truthy check (!localStorage.tutorials_tour_done), which works identically for both the old boolean (coerced to string"true"by localStorage) and the new explicit string. The code is correct as written.
| export interface DriverInstance { | ||
| highlight: (options: { | ||
| element: string | ||
| showButtons: boolean | ||
| popover: Popover | ||
| }) => void | ||
| defineSteps: (steps: TourStep[]) => void | ||
| start: () => void | ||
| reset: (clearHighlighted?: boolean) => void | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify interface matches driver.js API.
The DriverInstance interface appears well-structured. However, since this types a third-party library (driver.js), ensure the method signatures align with the actual library API to avoid runtime issues.
🌐 Web query:
driver.js library API documentation methods
💡 Result:
Here are the public methods on a driver.js instance (from the official API docs) with a short note for each: [1]
- drive([index]) — start the tour (optional start index). [1]
- moveNext() — go to next step. [1]
- movePrevious() — go to previous step. [1]
- moveTo(index) — go to a specific step. [1]
- hasNextStep() — returns whether a next step exists. [1]
- hasPreviousStep() — returns whether a previous step exists. [1]
- isFirstStep() — true if current step is first. [1]
- isLastStep() — true if current step is last. [1]
- getActiveIndex() — returns active step index. [1]
- getActiveStep() — returns active step configuration. [1]
- getPreviousStep() — returns previous step configuration. [1]
- getActiveElement() — returns currently highlighted DOM element. [1]
- getPreviousElement() — returns previously highlighted DOM element. [1]
- isActive() — whether a tour/highlight is currently active. [1]
- refresh() — recalc/redraw the highlight/popover. [1]
- getConfig() — get current config. [1]
- setConfig(config) — update configuration. [1]
- setSteps(steps) — set/replace the tour steps. [1]
- getState() — get driver state (see config/state docs). [1]
- highlight(step) — highlight a single element / show a popover. [1]
- destroy() — destroy the tour and cleanup. [1]
Source: driver.js API Reference. [1]
[1] https://driverjs.com/docs/api
Critical mismatches between DriverInstance interface and actual driver.js API.
The interface methods do not align with the official driver.js library:
highlight(options: {...})— driver.js expectshighlight(step), not an options object with element/showButtons/popoverdefineSteps(steps)— actual method issetSteps(steps)start()— actual method isdrive([index])reset(clearHighlighted?)— no such method;destroy()is the cleanup method
Update method names and signatures to match the actual driver.js API to prevent runtime errors.
🤖 Prompt for AI Agents
In src/simulator/src/types/tutorials.types.ts around lines 15 to 24, the
DriverInstance interface methods and signatures don't match driver.js: rename
and update signatures so they mirror the real API — replace
highlight(options:{...}) with highlight(step: TourStep), change
defineSteps(steps: TourStep[]) to setSteps(steps: TourStep[]), replace start()
with drive(index?: number) and remove reset(clearHighlighted?) in favor of
destroy(): void; ensure Popover/other types referenced align with TourStep
properties so the TypeScript types reflect driver.js usage.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/simulator/src/types/tutorials.types.ts (1)
21-30: Critical: DriverInstance interface still doesn't match driver.js API.The interface methods remain misaligned with the actual driver.js library API, as previously flagged. This will cause runtime errors:
start()→ should bedrive(index?: number): voiddefineSteps(steps: TourStep[])→ should besetSteps(steps: TourStep[]): voidreset(clearHighlighted?: boolean)→ should bedestroy(): voidhighlight(options: {...})→ should behighlight(step: TourStep): voidBased on the previous review's web search results, update the interface to match the official driver.js API:
export interface DriverInstance { - start(): void - defineSteps(steps: TourStep[]): void - highlight(options: { - element: string - popover: Popover - showButtons?: boolean - }): void - reset(clearHighlighted?: boolean): void + drive(index?: number): void + setSteps(steps: TourStep[]): void + highlight(step: TourStep): void + destroy(): void + moveNext(): void + movePrevious(): void + isActive(): boolean }
| export interface TourStep { | ||
| element: string | ||
| className?: string | ||
| popover: { | ||
| className?: string | ||
| title: string | ||
| description: string | ||
| position: 'top' | 'bottom' | 'left' | 'right' | ||
| offset?: number | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify TourStep structure matches driver.js step configuration.
Ensure the TourStep interface aligns with driver.js's expected step configuration, particularly the element selector format and popover structure.
🌐 Web query:
driver.js step configuration structure
💡 Result:
A Driver.js "step" (DriveStep) is an object you pass in the driver config steps array or to highlight(). Main properties:
- element: Element | string (CSS selector) | () => Element — target to highlight.
- popover: object controlling the popover:
- title?: string
- description?: string
- side?: "top" | "right" | "bottom" | "left"
- align?: "start" | "center" | "end"
- showButtons?: ("next" | "previous" | "close")[]
- callbacks: onNextClick, onPrevClick, onCloseClick (per-popover).
- disableActiveInteraction?: boolean — prevents interaction with target.
- per-step hooks/callbacks: onDeselected, onHighlightStarted, onHighlighted (receive element, step, options).
Example:
const steps = [
{
element: '.page-header',
popover: {
title: 'Header',
description: 'This is the header.',
side: 'bottom',
align: 'center',
showButtons: ['next','close'],
onNextClick: () => { /* custom logic / }
},
disableActiveInteraction: false,
onHighlighted: (el, step, opts) => { / ... */ }
}
];
(Driver-level config and additional options like animate, overlay color, global hooks, and state access are also available.) [1][2]
Sources:
[1] Driver.js — Configuration (DriveStep & Popover types).
[2] Driver.js — Basic usage / steps example.
Critical: TourStep interface has multiple API mismatches with driver.js DriveStep structure.
The TourStep interface does not align with driver.js's actual step configuration. Key issues:
- Property
positionshould beside(valid values: "top" | "right" | "bottom" | "left") - Missing
alignproperty ("start" | "center" | "end") - Missing
showButtonsarray for controlling button visibility - Missing
disableActiveInteractionflag - Missing popover callbacks (
onNextClick,onPrevClick,onCloseClick) - Missing step-level hooks (
onHighlighted,onDeselected,onHighlightStarted) elementshould acceptElement | string | (() => Element), not juststringpopover.titleanddescriptionshould be optional (?:), not required- Remove non-standard properties:
className(both levels) andoffset
Update the interface to match driver.js API specifications to prevent runtime errors and ensure proper type safety.
🤖 Prompt for AI Agents
In src/simulator/src/types/tutorials.types.ts around lines 1 to 11, the TourStep
interface does not match driver.js DriveStep; update it to: change property
position -> side with allowed values "top" | "right" | "bottom" | "left"; add
optional align?: "start" | "center" | "end"; add optional showButtons?: string[]
(or a typed enum/tuple matching driver.js buttons) and optional
disableActiveInteraction?: boolean; change element type to Element | string |
(() => Element); remove className and offset properties at both step and popover
levels; make popover.title? and popover.description? optional; add optional
popover callbacks onNextClick?, onPrevClick?, onCloseClick? and add optional
step-level hooks onHighlighted?, onDeselected?, onHighlightStarted? so the
interface matches driver.js API and avoids runtime type mismatches.
1 participant
Fixes #661
Describe the changes you have made in this PR -
refactored tutorial.js and sequential.js to ts.
Screenshots of the changes (If any) -
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit
Refactor
Documentation
Note: No user-facing behavior or functionality was changed.