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

add onLocationChanged method typings #248

Merged

Conversation

goszczynskip
Copy link
Contributor

This PR will add missing typescript typings for exposed onLocationChange method. Issue: #246

@salguerooo
Copy link
Contributor

@goszczynskip Even though this actions is being exported on the actions files, it is not made public for users of the library, it is for internal use, as can be seen on this line of the index.js
export { LOCATION_CHANGE, CALL_HISTORY_METHOD, push, replace, go, goBack, goForward, routerActions } from "./actions"
it shouldn't be typed as it is not possible to use it.

@goszczynskip
Copy link
Contributor Author

goszczynskip commented Jan 24, 2019

@salguerooo If you look at snippet below you'll see that it's exported in index.js file.

export { LOCATION_CHANGE, CALL_HISTORY_METHOD, onLocationChanged, push, replace, go, goBack, goForward, routerActions } from "./actions"

I'm listening for LOCATION_CHANGE action in redux-observable epic. It'd be nice if I could use onLocationChange action creator in my typescript tests. Since typings are't reflecting exported members my tests would fail. Of course I can type it in my d.ts files but I think it should be done here.

@salguerooo
Copy link
Contributor

@goszczynskip My bad then! I was referencing some local outdated code.

@supasate
Copy link
Owner

supasate commented Feb 5, 2019

LGTM. Thanks for your contribution!

@supasate supasate merged commit 14e7b11 into supasate:master Feb 5, 2019
@nikolay-borzov
Copy link

nikolay-borzov commented Feb 10, 2019

I believe you cannot use parameter initializer (isFirstRendering: boolean = false) inside a definition file.
TS complains:

A parameter initializer is only allowed in a function or constructor implementation

@goszczynskip
Copy link
Contributor Author

@nikolay-borzov Yes, you are right. I can see that fix is waiting for merge so I won't create new one.

I'm sorry for any trouble caused by this.

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

Successfully merging this pull request may close these issues.

4 participants