-
Notifications
You must be signed in to change notification settings - Fork 45
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.
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
Add type declarations #1259
base: main
Are you sure you want to change the base?
Add type declarations #1259
Conversation
charlesfries
commented
Dec 8, 2022
- Adds TypeScript ambient types
declare module 'ember-shepherd/services/tour' { | ||
import Shepherd from 'shepherd.js'; | ||
|
||
export interface TourButton extends Shepherd.Step.StepOptionsButton { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This interface isn't really part of the tour service, but is needed because the default Shepard.js button type does not have a type
property
index.d.ts
Outdated
title: string; | ||
} | ||
|
||
export default interface Tour { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@charlesfries could we import these types from Shepherd itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Tour
class type here https://github.com/shipshapecode/shepherd/blob/master/src/types/tour.d.ts seems to be pretty similar, but there are methods like addStep()
that exist in the Shepherd Tour
class that don't exist in the Tour
Ember service.
Definitely open to figuring out how to use those types and avoid duplication, but IMO the Tour
object and Tour
service may need to be distinct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@charlesfries I'm not super experienced with TS, but I was hoping we could merge them into one. Perhaps the ember-shepherd one can extend the Shepherd ones and add more things?