Skip to content
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

Types don't support activeTour or Evented properties. #504

Closed
mfallihee opened this issue Aug 12, 2019 · 4 comments · Fixed by #507
Closed

Types don't support activeTour or Evented properties. #504

mfallihee opened this issue Aug 12, 2019 · 4 comments · Fixed by #507

Comments

@mfallihee
Copy link

I have a very simple test with the current version 4.6.0:

import Shepherd from "shepherd.js";

let activeTour = Shepherd.activeTour; 
let evented = new Shepherd.Evented();

This will result in two errors on Typescript@3.5.3

error TS2339: Property 'activeTour' does not exist on type 'typeof Shepherd'
error TS2339: Property 'Evented' does not exist on type 'typeof Shepherd'

Additionally the current types will let few things will compile which should not:

let activeTour = new Shepherd().activeTour;
new Shepherd().on("event", function() { ... });

It looks to me that these errors are mostly related to this recent commit 9466a97

My suggestion for a fix that restores the documented behavior:

// shepherd.d.ts
import _Evented from './evented';
import _Step from './step';
import _Tour from './tour';

declare namespace Shepherd {
    export import Tour = _Tour;
    export import Step = _Step;
    export import Evented = _Evented;
    export const activeTour: _Tour | undefined;

    export function on(event: string, handler: Function): void;
    export function once(event: string, handler: Function): void;
    export function off(event: string, handler: Function): boolean | void;
    export function trigger(event: string): void;
}

export default Shepherd;

This also requires adding an empty namespace to evented.d.ts.

declare namespaces Evented { }

Unfortunately I could not find any way to get the desired result without duplicating the Evented interface which seems to be related to this limitation in Typescript: #18163.

@RobbieTheWagner
Copy link
Member

@mfallihee where are you using typeof Shepherd? If you use Shepherd as the type instead of typeof Shepherd I believe things should work.

@mfallihee
Copy link
Author

@rwwagner90 This is the snippet generating the errors:

import Shepherd from "shepherd.js";

let activeTour = Shepherd.activeTour; 
let evented = new Shepherd.Evented();

I am not explicitly using "typeof Shepherd" anywhere but that is the resulting type of the import when you reference Shepherd as a value.

If I do what you are suggesting like so and explicitly cast to "Shepherd" using it as a type, it does work:

import Shepherd from "shepherd.js";
let Shepherd2: Shepherd = <any>Shepherd;

let activeTour = Shepherd2.activeTour;
Shepherd2.on("event", function() {});

However, this seems far from ideal and not what was intended.

@RobbieTheWagner
Copy link
Member

@mfallihee the activeTour one we definitely need to fix, but the Evented one I am not sure about. I don't think people should be using Shepherd.Evented. It's used for events on tours and steps.

@mfallihee
Copy link
Author

@rwwagner90 That sounds good to me. I'm indifferent to the Shepherd.Evented one since I was not actually using it.
Probably best to just update the documentation to not include the Shepherd.Evented usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants