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

fix: Add some type definitions #459

Merged
merged 3 commits into from
Aug 5, 2019
Merged

fix: Add some type definitions #459

merged 3 commits into from
Aug 5, 2019

Conversation

hwwi
Copy link
Contributor

@hwwi hwwi commented Jul 18, 2019

Sorry about frequently open a PR.
But createMatchEnhancer was bothering me 😫

@taion
Copy link
Contributor

taion commented Jul 18, 2019

No, thank you for adding these!

@taion
Copy link
Contributor

taion commented Jul 18, 2019

If it's not too much to ask, would you mind adding a server/test.tsx analogous to https://github.com/4Catalyzer/found/blob/6f55794da28f1ef265d24502b426912f38ff48a1/types/test.tsx just to demonstrate the type definitions in action?

It will also keep us from accidentally breaking stuff.

@hwwi
Copy link
Contributor Author

hwwi commented Jul 18, 2019

First, Sorry for my poor English.

I think that getFarceResult function's return type is not Discriminated Unions, so infer is hard.

So I used it this way...

interface FarceResult {
    status?: number;
    element?: React.ReactElement;
    redirect?: {
        url: string;
    };
}

For the time being, I have excluded it from writing test code.
If you decide the direction, I will write continue.

@hwwi
Copy link
Contributor Author

hwwi commented Aug 5, 2019

Any comments or thoughts on this?

@taion
Copy link
Contributor

taion commented Aug 5, 2019

Sorry for the delay. I'll try to look at this and 4Catalyzer/farce#175 today.

@taion taion merged commit 466e39c into 4Catalyzer:master Aug 5, 2019
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