Skip to content

Flow-Router Extra Typescript types#95

Merged
dr-dimitru merged 6 commits intoveliovgroup:masterfrom
timsun28:master
Sep 19, 2022
Merged

Flow-Router Extra Typescript types#95
dr-dimitru merged 6 commits intoveliovgroup:masterfrom
timsun28:master

Conversation

@timsun28
Copy link
Copy Markdown

As discussed in issue there was an old request to add typescript types to this package.

This is a first start for implementing typescript types for Flow-Router extra. Basic setup from monti-apm routes types, but updated according to the docs from Flow-Router-Extra.

I still have a few small questions and a bug in the code itself that I wasn't able to figure out and could use the help from someone else with.

On the FlowRouter.current() documentation page, it is mentioned that the return value is an Object with 4 params, but there are more (as seen in the types file).

Later in the docs on the triggersEnter page, it is mentioned that the function has a property called: context {Route} that is equal to FlowRouter.current() function.
But the FlowRouter.current() function has a context prop that it returns with different props. I think my implementation for the types is correct, but this might be a separate issue with the documentation that could be improved.

My issue is with the dynamic import (return type for waitOn function).
type DynamicImport = Promise<typeof import(T)>;
I have tried the following solution, but it's giving the error message at the last T: String literal expected.

Copy link
Copy Markdown
Member

@dr-dimitru dr-dimitru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timsun28 looks great, thank you fro the contribution.
Is there a way to cover it with tests?

@dr-dimitru dr-dimitru merged commit 7c1b9f1 into veliovgroup:master Sep 19, 2022
dr-dimitru added a commit that referenced this pull request Sep 19, 2022
__Changes:__

- 👷‍ Merge: #95 - Typescript types by @timsun28 closing #84

__Dependencies__:

- 📦 `qs@6.11.0`, *was `v6.10.5`*
@timsun28
Copy link
Copy Markdown
Author

timsun28 commented Sep 19, 2022

@dr-dimitru Thank you for merging this PR.

[Update] I've removed the local package from my project and update FlowRouter and it successfully found all the types!

@dr-dimitru
Copy link
Copy Markdown
Member

@timsun28 I'm suggesting to start with a small file making sure types are pulled and recognized

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.

2 participants