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

Exported external package typings file 'redux-mock-store/index.d.ts' is not a module. Please contact the package author to update the package definition #39

Closed
brauliodiez opened this issue May 22, 2016 · 13 comments

Comments

@brauliodiez
Copy link
Contributor

brauliodiez commented May 22, 2016

I have downloaded redux-mock-store and it include a definition, when you attempt to use it provides the following error:

Exported external package typings file 'C:/Test/node_modules/redux-mock-store/index.d.ts' is not a module. Please contact the package author to update the package definition

We have implemented a manual typing for redux-mock-store, that worked fine for us:

/// <reference path="../../typings/globals/redux/index.d.ts"/>

declare module "redux-mock-store" {
    interface MockStore extends Redux.Store {
        getState(): any;
        getActions(): Array<any>;
        dispatch(action: any): any;
        clearActions(): void;
        subscribe(): any;
    }

    function configureStore(...args: any[]) : (...args: any[]) => MockStore;
    export = configureStore;
}

This index.d.ts is providing conflicts (even excluding node_modules folders), in our case we are manually deleting that file whenever we install the package.Could it be a good idea to remove the d.ts from redux-mock-store, review it and an updated on to the typings repository?

@arnaudbenard
Copy link
Contributor

. @dylanparry implemented the TypeScript part of redux-mock-store, can you check with him on how to fix it?

I have never used TS in the past.

@dylanparry
Copy link
Contributor

Hi. It looks like the API for Redux Mock Store has changed considerably since I wrote the index.d.ts file. For now it's probably best to either replace the file with the code that @brauliodiez has included above, or delete the file reference from the package.json until it can be updated correctly.

@kenjiru
Copy link

kenjiru commented May 24, 2016

Same issue here, the typings file is broken.

@brauliodiez
Copy link
Contributor Author

May I make a fork and ask for pull request?

@arnaudbenard
Copy link
Contributor

@brauliodiez Feel free to make a PR 👍
@dylanparry Thank you for the feedback

@brauliodiez
Copy link
Contributor Author

I have given a try and there is something wrong, if I place the file outside node_modules (as a typing) works fine, but inside node_modules I get the same error again, I will take look.

@asgarddesigns
Copy link
Contributor

@brauliodiez Did you find out what the problem is there? Facing the same issue.

@asgarddesigns
Copy link
Contributor

asgarddesigns commented May 30, 2016

So after a bit of searching I came across this which seems to indicate we shouldn't be declaring a top level module for an external definition.

Bear in mind I'm pretty new to a lot of this so I may be missing something (or many things).

Changing to the following works for me:

import {Store} from 'redux';

interface MockStore extends Store {
    getState():any;
    getActions():Array<any>;
    dispatch(action:any):any;
    clearActions():void;
    subscribe():any;
}

declare function configureStore(...args:any[]):(...args:any[]) => MockStore;
export = configureStore;

i.e. the top level module has been removed and the function is a declare because "'declare' [is] now required for top level non-interface declarations " - Typescript - Known breaking changes between 0.8 and 0.9

Let me know what you think, I can create a PR if it's needed (and this is correct!).

@brauliodiez
Copy link
Contributor Author

You are right, we have checked and you version is working like a charm. @dylanparry , @arnaudbenard should @asgarddesigns raise a Pull Request includin the fix for the breaking change?

@mfrachet
Copy link

Anybody for this PR ?

@arnaudbenard
Copy link
Contributor

Is everyone happy with this change?

@dylanparry
Copy link
Contributor

Yes. Seems to work fine, so go with it.

On Wed, Jun 15, 2016 at 2:52 PM +0100, "Arnaud Bénard" notifications@github.com wrote:

Is everyone happy with this change?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@arnaudbenard
Copy link
Contributor

Done! Thanks everyone for the contribution 👍

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

No branches or pull requests

6 participants