jeremybanka / wayforge

TypeScript monorepo. Home of Atom.io.
https://atom.io.fyi
2 stars 2 forks source link

atom.io json entries #2386

Closed jeremybanka closed 1 month ago

jeremybanka commented 1 month ago

User description


PR Type

Enhancement, Tests, Documentation


Description


Changes walkthrough πŸ“

Relevant files
Tests
entries.test.ts
Add tests for `fromEntries` function in `atom.io/json`     

packages/atom.io/__tests__/public/json/entries.test.ts
  • Added a test for fromEntries function.
  • Ensured type-safe conversion of entries array to an object.
  • +14/-0   
    Enhancement
    entries.ts
    Implement `Entries` type and `fromEntries` function           

    packages/atom.io/json/src/entries.ts
  • Introduced Entries type for handling key-value pairs.
  • Added fromEntries function to convert entries array to an object.
  • Defined FromEntries type for type-safe conversion.
  • +21/-0   
    index.ts
    Export `entries` module in `atom.io/json`                               

    packages/atom.io/json/src/index.ts - Exported `entries` module from `atom.io/json`.
    +1/-0     
    Documentation
    thirty-cameras-invent.md
    Document new `Entries` type and `fromEntries` function     

    .changeset/thirty-cameras-invent.md
  • Documented the addition of Entries type and fromEntries function.
  • +5/-0     

    πŸ’‘ PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    vercel[bot] commented 1 month ago

    The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

    Name Status Preview Comments Updated (UTC)
    atom-io-fyi βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Aug 12, 2024 7:07am
    wayfarer-quest βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Aug 12, 2024 7:07am
    changeset-bot[bot] commented 1 month ago

    πŸ¦‹ Changeset detected

    Latest commit: b93a14f2a61209b61cf0dde0254f1b83da5f24b4

    The changes in this PR will be included in the next version bump.

    This PR includes changesets to release 1 package | Name | Type | | ------- | ----- | | atom.io | Patch |

    Not sure what this means? Click here to learn what changesets are.

    Click here if you're a maintainer who wants to add another changeset to this PR

    github-actions[bot] commented 1 month ago

    PR Reviewer Guide πŸ”

    ⏱️ Estimated effort to review: 2 πŸ”΅πŸ”΅βšͺβšͺβšͺ
    πŸ§ͺ PR contains tests
    πŸ”’ No security concerns identified
    ⚑ No key issues to review
    github-actions[bot] commented 1 month ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add error handling for duplicate keys in fromEntries ___ **Add error handling in fromEntries to manage non-unique keys in the entries array,
    which could lead to unexpected behavior.** [packages/atom.io/json/src/entries.ts [19-21]](https://github.com/jeremybanka/wayforge/pull/2386/files#diff-ff3bc315bc8304e0eca12bcbb3917505ed90ddd633661414e0e866c1030b5351R19-R21) ```diff export function fromEntries(entries: E): FromEntries { + const keys = new Set(entries.map(entry => entry[0])) + if (keys.size !== entries.length) { + throw new Error("Duplicate keys found in entries."); + } return Object.fromEntries(entries) as FromEntries } ```
    Suggestion importance[1-10]: 10 Why: Adding error handling for duplicate keys prevents potential bugs and ensures the function behaves predictably, which is crucial for data integrity.
    10
    Ensure fromEntries handles more than ten entries appropriately ___ **Modify the fromEntries function to handle cases where the entries array exceeds ten
    items, as the current implementation limits it to up to ten entries.** [packages/atom.io/json/src/entries.ts [19-21]](https://github.com/jeremybanka/wayforge/pull/2386/files#diff-ff3bc315bc8304e0eca12bcbb3917505ed90ddd633661414e0e866c1030b5351R19-R21) ```diff export function fromEntries(entries: E): FromEntries { + if (entries.length > 10) { + throw new Error("fromEntries can only handle up to ten entries."); + } return Object.fromEntries(entries) as FromEntries } ```
    Suggestion importance[1-10]: 8 Why: Adding a check for the number of entries ensures that the function does not silently fail or produce incorrect results when the input exceeds the expected limit.
    8
    Enhancement
    Add a test case for empty array input to fromEntries ___ **Consider adding a test case to handle scenarios where fromEntries receives an empty
    array. This will ensure that the function behaves as expected even with no entries.** [packages/atom.io/__tests__/public/json/entries.test.ts [4-13]](https://github.com/jeremybanka/wayforge/pull/2386/files#diff-ba150176b710aa24903e4f2d632e8bb3dd79cf2eaa9d40236cda5f5ab3d64de1R4-R13) ```diff describe(`fromEntries`, () => { it(`type-safely converts an array of entries to an object`, () => { const myEntries = [ [`a`, 1], [`b`, 1], [`c`, 1], ] satisfies Entries const { a, b, c } = fromEntries(myEntries) expect(a + b + c).toBe(3) }) + it(`handles empty entries array`, () => { + const myEntries: Entries = [] + const result = fromEntries(myEntries) + expect(result).toEqual({}) + }) }) ```
    Suggestion importance[1-10]: 9 Why: Adding a test case for an empty array input ensures that the function handles edge cases correctly, improving the robustness of the code.
    9
    Simplify the CertainEntry type definition for better readability and efficiency ___ **Refactor the CertainEntry type to use a simpler and more efficient type definition
    that avoids the complex mapping over UpToTen.** [packages/atom.io/json/src/entries.ts [11-13]](https://github.com/jeremybanka/wayforge/pull/2386/files#diff-ff3bc315bc8304e0eca12bcbb3917505ed90ddd633661414e0e866c1030b5351R11-R13) ```diff -export type CertainEntry> = { - [P in UpToTen]: E[P] extends [K, infer V] ? V : never -}[UpToTen] +export type CertainEntry> = E extends [K, infer V][] ? V : never ```
    Suggestion importance[1-10]: 7 Why: The refactor improves readability and maintainability of the type definition, though it does not address a functional issue.
    7
    coveralls commented 1 month ago

    Coverage Status

    coverage: 91.544% (+0.003%) from 91.541% when pulling b93a14f2a61209b61cf0dde0254f1b83da5f24b4 on atom.io-json-entries into ae2ed9a7024ca9b9ae809931afaeada06885da2b on main.