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

update type definition #97

Merged
merged 3 commits into from Nov 19, 2019
Merged

update type definition #97

merged 3 commits into from Nov 19, 2019

Conversation

cbbfcd
Copy link
Contributor

@cbbfcd cbbfcd commented Nov 18, 2019

update type definition, because we have basepath now!

types/index.d.ts Outdated
@@ -62,6 +62,7 @@ export const Switch: FunctionComponent<SwitchProps>;

export interface RouterProps {
hook: LocationHook;
basepath: string;
Copy link
Owner

Choose a reason for hiding this comment

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

While it's not really critical, but I would suggest using a Path type here (which is just an alias to string, but helps to keep the code more generic I guess). Basically Path represents something that can describe a location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea!! Thx! i will fix it

Copy link
Owner

@molefrog molefrog left a comment

Choose a reason for hiding this comment

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

Nice work! Just one small comment regarding the Path type and we can merge this.

@omgovich
Copy link
Contributor

@cbbfcd thank you very much! You help us a lot

@codecov-io
Copy link

codecov-io commented Nov 19, 2019

Codecov Report

Merging #97 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
- Coverage   99.45%   99.44%   -0.01%     
==========================================
  Files           7        7              
  Lines         182      181       -1     
  Branches       32       32              
==========================================
- Hits          181      180       -1     
  Misses          1        1
Impacted Files Coverage Δ
index.js 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0b518f...ed27644. Read the comment docs.

@molefrog molefrog merged commit 85b0177 into molefrog:master Nov 19, 2019
@cbbfcd cbbfcd deleted the types-add branch November 19, 2019 08:01
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.

None yet

4 participants