-
Notifications
You must be signed in to change notification settings - Fork 0
⚡ Optimize getLanguageColor performance #287
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| import { describe, it, expect, vi } from "vitest"; | ||
| import { fetchUserSummary } from "@/lib/github"; | ||
| import { jsonResponse } from "./setup"; | ||
|
|
||
| describe("getTopK logic via fetchUserSummary", () => { | ||
| it("covers getTopK branch for when the array is already filled to k and needs to insert a new element (shifting)", async () => { | ||
| // We want to return 15 repositories, each with a different language, and byte count such that it triggers shifting. | ||
| // getTopK is used in fetchRepositories for languages and topics. | ||
| // Let's create a custom GraphQL response. | ||
| const nodes = []; | ||
| // If we want it to shift: | ||
| // First, populate the top 10 with values [10, 20, 30, 40, 50, 60, 70, 80, 90, 100]. | ||
| // Then, add a value like `75`. This should shift the lower items. | ||
|
|
||
| // We'll create 15 repositories. | ||
| const sizes = [10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 75, 15, 85, 25, 95]; | ||
| for (let i = 0; i < sizes.length; i++) { | ||
| nodes.push({ | ||
| id: `id${i}`, | ||
| name: `repo${i}`, | ||
| description: null, | ||
| url: `https://github.com/u/repo${i}`, | ||
| isFork: false, | ||
| stargazerCount: 0, | ||
| forkCount: 0, | ||
| languages: { | ||
| edges: [ | ||
| { | ||
| size: sizes[i], | ||
| node: { | ||
| name: `Lang${i}`, | ||
| color: "#000000" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| repositoryTopics: { nodes: [] } | ||
| }); | ||
| } | ||
|
|
||
| const mockReposGraphQL = { | ||
| data: { | ||
| user: { | ||
| repositories: { | ||
| nodes, | ||
| pageInfo: { hasNextPage: false, endCursor: null } | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| const mockFetch = vi.fn().mockImplementation((url: string | URL | Request, options?: RequestInit) => { | ||
| const urlStr = typeof url === "string" ? url : url instanceof URL ? url.toString() : (url as Request).url; | ||
| if (urlStr.includes("/graphql")) { | ||
| const body = options?.body ? JSON.parse(options.body as string) : {}; | ||
| if (body.query?.includes("repositories")) { | ||
| return Promise.resolve(jsonResponse(mockReposGraphQL)); | ||
| } | ||
| return Promise.resolve(jsonResponse({ data: {} })); | ||
| } | ||
| return Promise.resolve(jsonResponse({})); | ||
| }); | ||
|
|
||
| global.fetch = mockFetch; | ||
|
|
||
| const result = await fetchUserSummary("testuser", "fake-token"); | ||
| expect(result.repositories).not.toBeNull(); | ||
| // Languages should be top 10 | ||
| expect(result.repositories?.languages.length).toBe(10); | ||
| // The top languages should be sorted by size: | ||
| // 100, 95, 90, 85, 80, 75, 70, 60, 50, 40 | ||
| expect(result.repositories?.languages[0].bytes).toBe(100); | ||
| expect(result.repositories?.languages[5].bytes).toBe(75); | ||
| expect(result.repositories?.languages[9].bytes).toBe(40); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -820,45 +820,46 @@ export async function fetchUserSummary( | |||||
|
|
||||||
| // ===== ユーティリティ ===== | ||||||
|
|
||||||
| const LANGUAGE_COLORS: Record<string, string> = { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
| JavaScript: "#f1e05a", | ||||||
| TypeScript: "#3178c6", | ||||||
| Python: "#3572A5", | ||||||
| Java: "#b07219", | ||||||
| Go: "#00ADD8", | ||||||
| Rust: "#dea584", | ||||||
| "C++": "#f34b7d", | ||||||
| C: "#555555", | ||||||
| "C#": "#178600", | ||||||
| Ruby: "#701516", | ||||||
| PHP: "#4F5D95", | ||||||
| Swift: "#F05138", | ||||||
| Kotlin: "#A97BFF", | ||||||
| Dart: "#00B4AB", | ||||||
| Scala: "#c22d40", | ||||||
| Shell: "#89e051", | ||||||
| HTML: "#e34c26", | ||||||
| CSS: "#563d7c", | ||||||
| Vue: "#41b883", | ||||||
| Svelte: "#ff3e00", | ||||||
| Lua: "#000080", | ||||||
| R: "#198CE7", | ||||||
| Elixir: "#6e4a7e", | ||||||
| Haskell: "#5e5086", | ||||||
| Clojure: "#db5855", | ||||||
| Erlang: "#B83998", | ||||||
| Zig: "#ec915c", | ||||||
| Nim: "#ffc200", | ||||||
| OCaml: "#3be133", | ||||||
| Julia: "#a270ba", | ||||||
| Perl: "#0298c3", | ||||||
| Jupyter: "#DA5B0B", | ||||||
| "Jupyter Notebook": "#DA5B0B", | ||||||
| Dockerfile: "#384d54", | ||||||
| Makefile: "#427819", | ||||||
| HCL: "#844FBA", | ||||||
| Nix: "#7e7eff", | ||||||
| }; | ||||||
|
|
||||||
| function getLanguageColor(language: string): string { | ||||||
| const colors: Record<string, string> = { | ||||||
| JavaScript: "#f1e05a", | ||||||
| TypeScript: "#3178c6", | ||||||
| Python: "#3572A5", | ||||||
| Java: "#b07219", | ||||||
| Go: "#00ADD8", | ||||||
| Rust: "#dea584", | ||||||
| "C++": "#f34b7d", | ||||||
| C: "#555555", | ||||||
| "C#": "#178600", | ||||||
| Ruby: "#701516", | ||||||
| PHP: "#4F5D95", | ||||||
| Swift: "#F05138", | ||||||
| Kotlin: "#A97BFF", | ||||||
| Dart: "#00B4AB", | ||||||
| Scala: "#c22d40", | ||||||
| Shell: "#89e051", | ||||||
| HTML: "#e34c26", | ||||||
| CSS: "#563d7c", | ||||||
| Vue: "#41b883", | ||||||
| Svelte: "#ff3e00", | ||||||
| Lua: "#000080", | ||||||
| R: "#198CE7", | ||||||
| Elixir: "#6e4a7e", | ||||||
| Haskell: "#5e5086", | ||||||
| Clojure: "#db5855", | ||||||
| Erlang: "#B83998", | ||||||
| Zig: "#ec915c", | ||||||
| Nim: "#ffc200", | ||||||
| OCaml: "#3be133", | ||||||
| Julia: "#a270ba", | ||||||
| Perl: "#0298c3", | ||||||
| Jupyter: "#DA5B0B", | ||||||
| "Jupyter Notebook": "#DA5B0B", | ||||||
| Dockerfile: "#384d54", | ||||||
| Makefile: "#427819", | ||||||
| HCL: "#844FBA", | ||||||
| Nix: "#7e7eff", | ||||||
| }; | ||||||
| return colors[language] ?? "#8b949e"; | ||||||
| return LANGUAGE_COLORS[language] ?? "#8b949e"; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Directly indexing a plain object with a string from an external source (like the GitHub API) can lead to unexpected results if the string matches a property on Object.prototype (e.g., "toString", "constructor", or "hasOwnProperty"). In such cases, the function would return a function reference instead of a color string, which could cause runtime errors when applied to UI styles. Using Object.prototype.hasOwnProperty.call() or Object.hasOwn() ensures that only explicitly defined keys are matched.
Suggested change
|
||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LANGUAGE_COLORSはモジュールスコープに移動されましたが、型がRecord<string, string>のままで書き換え可能な状態です。as constを使うことで TypeScript がこのオブジェクトを読み取り専用のリテラル型として扱い、誤って値が変更されることを防げます。Prompt To Fix With AI