jeremybanka / wayforge

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

atom.io throw on family key conflict #2397

Closed jeremybanka closed 1 month ago

jeremybanka commented 1 month ago

User description


PR Type

Enhancement, Tests


Description


Changes walkthrough ๐Ÿ“

Relevant files
Tests
1 files
grace.test.ts
Add tests for family key conflict error handling                 

packages/atom.io/__tests__/private/grace.test.ts
  • Added test cases to ensure errors are thrown when creating families
    with duplicate keys.
  • Imported selectorFamily and SetRTX for new test cases.
  • +57/-0   
    Enhancement
    8 files
    create-readonly-selector-family.ts
    Handle key conflicts in readonly selector family creation

    packages/atom.io/internal/src/families/create-readonly-selector-family.ts
  • Added throwInCaseOfConflictingFamily function call to handle key
    conflicts.
  • Refactored family creation logic for readability.
  • +33/-26 
    create-regular-atom-family.ts
    Handle key conflicts in regular atom family creation         

    packages/atom.io/internal/src/families/create-regular-atom-family.ts
  • Added throwInCaseOfConflictingFamily function call to handle key
    conflicts.
  • Refactored family creation logic for readability.
  • +34/-23 
    create-writable-selector-family.ts
    Handle key conflicts in writable selector family creation

    packages/atom.io/internal/src/families/create-writable-selector-family.ts
  • Added throwInCaseOfConflictingFamily function call to handle key
    conflicts.
  • Refactored family creation logic for readability.
  • +34/-27 
    throw-in-case-of-conflicting-family.ts
    Add utility to throw errors on family key conflicts           

    packages/atom.io/internal/src/families/throw-in-case-of-conflicting-family.ts
  • Created new utility function to throw errors on family key conflicts.
  • +18/-0   
    index.ts
    Export prettyPrint utility                                                             

    packages/atom.io/internal/src/index.ts - Exported `prettyPrint` utility.
    +1/-0     
    create-mutable-atom-family.ts
    Handle key conflicts in mutable atom family creation         

    packages/atom.io/internal/src/mutable/create-mutable-atom-family.ts
  • Added throwInCaseOfConflictingFamily function call to handle key
    conflicts.
  • Refactored family creation logic for readability.
  • +36/-29 
    not-found-error.ts
    Refactor prettyPrintTokenType to separate file                     

    packages/atom.io/internal/src/not-found-error.ts
  • Moved prettyPrintTokenType to a separate file for better modularity.
  • +2/-35   
    pretty-print.ts
    Add prettyPrintTokenType utility function                               

    packages/atom.io/internal/src/pretty-print.ts - Created new file for `prettyPrintTokenType` function.
    +37/-0   
    Documentation
    1 files
    perfect-birds-double.md
    Add changeset for family key conflict error handling         

    .changeset/perfect-birds-double.md
  • Added changeset for the new error handling feature for family key
    conflicts.
  • +5/-0     

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

    changeset-bot[bot] commented 1 month ago

    ๐Ÿฆ‹ Changeset detected

    Latest commit: b702d2d90a2156f92a51c94ee0802e170dcac5c5

    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

    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 13, 2024 9:50pm
    wayfarer-quest โœ… Ready (Inspect) Visit Preview ๐Ÿ’ฌ Add feedback Aug 13, 2024 9:50pm
    github-actions[bot] commented 1 month ago

    PR Reviewer Guide ๐Ÿ”

    โฑ๏ธ Estimated effort to review: 3 ๐Ÿ”ต๐Ÿ”ต๐Ÿ”ตโšชโšช
    ๐Ÿงช PR contains tests
    ๐Ÿ”’ No security concerns identified
    โšก Key issues to review

    Test Coverage
    Ensure that the test cases cover all new error scenarios introduced for family key conflicts, including edge cases and different types of families. Error Handling
    Review the error handling logic to ensure that it correctly identifies and throws errors for all types of family conflicts, not just atom families.
    github-actions[bot] commented 1 month ago

    PR Code Suggestions โœจ

    CategorySuggestion                                                                                                                                    Score
    Error handling
    Implement error handling for selector creation ___ **Add error handling for the createWritableSelector function to manage exceptions that
    might occur during selector creation.** [packages/atom.io/internal/src/families/create-writable-selector-family.ts [41-49]](https://github.com/jeremybanka/wayforge/pull/2397/files#diff-cb43db3aca301de32b560837206d9ab8d7e46ea942867973aea122a720c31052R41-R49) ```diff -const token = createWritableSelector( - { - key: fullKey, - get: options.get(key), - set: options.set(key), - }, - family, - target, -) +let token; +try { + token = createWritableSelector( + { + key: fullKey, + get: options.get(key), + set: options.set(key), + }, + family, + target, + ); +} catch (error) { + console.error(`Failed to create writable selector: ${error}`); + throw error; +} ```
    Suggestion importance[1-10]: 9 Why: Adding error handling for the `createWritableSelector` function is crucial for managing exceptions and improving the reliability of the code.
    9
    Possible bug
    Add type safety check for function invocation ___ **Ensure that the type of def is checked before invoking it as a function to prevent
    runtime errors if def is not a function.** [packages/atom.io/internal/src/families/create-regular-atom-family.ts [44]](https://github.com/jeremybanka/wayforge/pull/2397/files#diff-1534b05697a5730de7721e61a41b1061226b23b32e4a8b8d80d5a5e6bb2caf9aR44-R44) ```diff -default: def instanceof Function ? def(key) : def, +default: typeof def === 'function' ? def(key) : def, ```
    Suggestion importance[1-10]: 8 Why: Ensuring the type of `def` is checked before invoking it as a function prevents potential runtime errors, enhancing code robustness.
    8
    Best practice
    Ensure options.effects is a function before invocation ___ **Consider using a more specific type check for options.effects to ensure it is a
    function before calling it, which enhances type safety and code robustness.** [packages/atom.io/internal/src/families/create-mutable-atom-family.ts [55-56]](https://github.com/jeremybanka/wayforge/pull/2397/files#diff-a96761c44810999842b2cc0f578b3a75493582fb9a59232db54a9645b39ef4a7R55-R56) ```diff -if (options.effects) { +if (typeof options.effects === 'function') { individualOptions.effects = options.effects(key) } ```
    Suggestion importance[1-10]: 8 Why: Using a more specific type check for `options.effects` enhances type safety and code robustness, which is a good practice.
    8
    Maintainability
    Refactor repeated error messages into a constant ___ **Consider refactoring the repeated error message into a constant to avoid duplication
    and facilitate easier maintenance or localization of error messages.** [packages/atom.io/__tests__/private/grace.test.ts [176-214]](https://github.com/jeremybanka/wayforge/pull/2397/files#diff-704681e68b526dd6ec35340de49c6e296c5f461e7ad23fc3c7bb75e249506176R176-R214) ```diff -`Tried to create an Atom Family with key "count", but "count" already exists in store "IMPLICIT_STORE" as an Atom Family` -`Tried to create a Readonly Selector Family with key "count", but "count" already exists in store "IMPLICIT_STORE" as an Atom Family` -`Tried to create a Mutable Atom Family with key "count", but "count" already exists in store "IMPLICIT_STORE" as an Atom Family` -`Tried to create a Selector Family with key "count", but "count" already exists in store "IMPLICIT_STORE" as an Atom Family` +const errorMessage = `Tried to create a family with key "count", but "count" already exists in store "IMPLICIT_STORE" as a family`; +... +}).toThrowError(errorMessage) +... +}).toThrowError(errorMessage) +... +}).toThrowError(errorMessage) +... +}).toThrowError(errorMessage) ```
    Suggestion importance[1-10]: 7 Why: Refactoring repeated error messages into a constant improves maintainability and readability, but it is not crucial for functionality.
    7
    coveralls commented 1 month ago

    Coverage Status

    coverage: 91.616% (+0.07%) from 91.544% when pulling b702d2d90a2156f92a51c94ee0802e170dcac5c5 on atom.io-throw-on-family-key-conflict into 3b7dde44dad75e0e26bdd72288aace2ab08579fc on main.