DevLeonardoCommunity / github-stats

Aggregating and displaying YOUR GitHub Stats in meaningful metrics (we learn web development in the meantime)
https://public-github-stats.vercel.app
MIT License
71 stars 37 forks source link

fix: export button placement in text mode #44

Closed jakubfronczyk closed 1 year ago

jakubfronczyk commented 1 year ago

Change button placement outside <pre> tag.

I spotted types problem. I used custom types from types/github.ts file.

vercel[bot] commented 1 year ago

Someone is attempting to deploy a commit to a Personal Account owned by @Balastrong on Vercel.

@Balastrong first needs to authorize it.

Balastrong commented 1 year ago

Thank you for the PR!

The fix looks good, but I don't see why we need to add those types explicitly. What errors did you get?

jakubfronczyk commented 1 year ago

I get "Binding element 'repository' implicitly has an any type. Error that occurs when TypeScript can't determine the type of a variable or parameter automatically. To address this issue, I added explicit type annotations using our custom types. By specifying the expected types for repository and contributions using the custom PullRequestContributionsByRepository type, we could enhance type-safety and catch potential type-related issues at compile time.

Balastrong commented 1 year ago

Thanks for the details! I'm asking because on visual studio code I already see the types there properly. Are you using some strict extension or a different editor?

repositories is already typed and it's an array, I'm assuming calling .map on it should infer the types of each element with no effort.

What happens if you explicitly type it on line 18 instead? Would that still need the extra typings in the .map?

  const {
    repositories,
    isLoading,
  }: {
    repositories: PullRequestContributionsByRepository[];
    isLoading: boolean;
  } = useGitHubPullRequests(year, login as string);
jakubfronczyk commented 1 year ago

I'm using the VSCode and if i'm not wrong it's not specifically related to an extension, when you have the TypeScript language service enabled the editor itself will highlight these errors.

Your apporach also works and we don't need extra typing in .map I'm thinking it is even better becasue also ensures type safety across your codebase wherever these variables are used.

Balastrong commented 1 year ago

That's great, thank you!

Then if that's also ok for you to move that type there I'll merge the PR after your commit :)

jakubfronczyk commented 1 year ago

I pushed changes.

Thank you :) It seems really fun project, I hope I could help more.

Balastrong commented 1 year ago

@all-contributors please add @jakubfronczyk for code

allcontributors[bot] commented 1 year ago

@Balastrong

I've put up a pull request to add @jakubfronczyk! :tada: