keyshade-xyz / keyshade

Realtime secret and configuration management tool, with the best in class security and seamless integration support
https://keyshade.xyz
Mozilla Public License 2.0
196 stars 96 forks source link

chore(platform): Added optional chaining due to strict null check #413

Closed unamdev0 closed 3 weeks ago

unamdev0 commented 3 weeks ago

User description

Description

Added optional chaining in platform code due to strict null check implementation

Fixes #412


PR Type

enhancement, bug_fix


Description


Changes walkthrough ๐Ÿ“

Relevant files
Enhancement
10 files
page.tsx
Add localStorage check and optional chaining                         

apps/platform/src/app/(main)/page.tsx
  • Added check for localStorage before accessing it.
  • Used optional chaining for safer JSON parsing.
  • +6/-3     
    page.tsx
    Use optional chaining for secrets mapping                               

    apps/platform/src/app/(main)/project/[project]/@secret/page.tsx - Added optional chaining to `allSecrets` mapping.
    +1/-1     
    layout.tsx
    Use optional chaining for project name access                       

    apps/platform/src/app/(main)/project/[project]/layout.tsx - Added optional chaining to access `currentProject.name`.
    +1/-1     
    page.tsx
    Use optional chaining for input focus                                       

    apps/platform/src/app/auth/invite/page.tsx - Added optional chaining to `inputRef.current` focus calls.
    +3/-3     
    index.tsx
    Use optional chaining for user name access                             

    apps/platform/src/components/shared/navbar/index.tsx - Added optional chaining to safely access `data.name`.
    +1/-1     
    index.tsx
    Use Boolean for conditional rendering                                       

    apps/platform/src/components/teams/teamTable/index.tsx - Used `Boolean` for conditional rendering logic.
    +1/-1     
    line-tab.tsx
    Use optional chaining for search comparison                           

    apps/platform/src/components/ui/line-tab.tsx - Added optional chaining for `search` comparison.
    +1/-1     
    env.ts
    Use optional chaining for error handling                                 

    apps/platform/src/env.ts - Added optional chaining to handle potential undefined errors.
    +1/-1     
    workspace-storage.ts
    Add localStorage checks for workspace storage                       

    apps/platform/src/lib/workspace-storage.ts - Added checks for `localStorage` before accessing it.
    +8/-3     
    middleware.ts
    Use optional chaining for cookie value access                       

    apps/platform/src/middleware.ts - Added optional chaining for cookie value access.
    +1/-1     
    Dependencies
    1 files
    package.json
    Update dependencies in package.json                                           

    package.json - Added `@keyshade/api-client` to dependencies.
    +2/-2     

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

    codiumai-pr-agent-free[bot] commented 3 weeks ago

    PR Reviewer Guide ๐Ÿ”

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

    Possible Bug
    The optional chaining operator is used on `allSecrets`, but it's not clear where this variable is defined or if it can be null/undefined. Unnecessary Boolean Conversion
    The `Boolean()` function is unnecessarily used on an already boolean expression. Inconsistent Null Checks
    The function checks for `localStorage` availability inconsistently. Some checks use `typeof localStorage !== 'undefined'`, while others use `typeof localStorage!=="undefined"`.
    codiumai-pr-agent-free[bot] commented 3 weeks ago

    PR Code Suggestions โœจ

    CategorySuggestion                                                                                                                                    Score
    Code organization
    โœ… Consolidate localStorage availability checks to reduce code duplication ___
    Suggestion Impact:The commit consolidated the localStorage availability check by removing the duplicate check within the nested if statement, aligning with the suggestion to reduce code duplication. code diff: ```diff if (typeof localStorage !== 'undefined') { localStorage.setItem('defaultWorkspace', JSON.stringify(defaultWorkspace)) - } - if (getCurrentWorkspace() === null) { - if (typeof localStorage !== 'undefined') { + + if (getCurrentWorkspace() === null) { localStorage.setItem('currentWorkspace', JSON.stringify(defaultWorkspace)) } } ```
    ___ **Consider using a single check for typeof localStorage !== 'undefined' at the
    beginning of the function to improve readability and reduce code duplication.** [apps/platform/src/lib/workspace-storage.ts [6-12]](https://github.com/keyshade-xyz/keyshade/pull/413/files#diff-3399b4a605e2a48dd3faa057150c25a91b298f00543cc21fc191839f08d236cbR6-R12) ```diff if (typeof localStorage !== 'undefined') { localStorage.setItem('defaultWorkspace', JSON.stringify(defaultWorkspace)) -} -if (getCurrentWorkspace() === null) { - if (typeof localStorage !== 'undefined') { + if (getCurrentWorkspace() === null) { localStorage.setItem('currentWorkspace', JSON.stringify(defaultWorkspace)) } } ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 9 Why: The suggestion improves code readability and organization by consolidating the `typeof localStorage !== 'undefined'` check, reducing code duplication and making the function more concise.
    9
    Simplification
    Simplify boolean expression by removing unnecessary Boolean() conversion ___ **The Boolean() function is unnecessary here as the table.getIsSomePageRowsSelected()
    already returns a boolean. Consider simplifying the expression.** [apps/platform/src/components/teams/teamTable/index.tsx [92]](https://github.com/keyshade-xyz/keyshade/pull/413/files#diff-aed7f921b4006dd809b3ffdf114a5eee8eaaa3e68fe789c8ad99a533fcc81b5cR92-R92) ```diff -Boolean(table.getIsSomePageRowsSelected() && 'indeterminate') +table.getIsSomePageRowsSelected() && 'indeterminate' ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 8 Why: The suggestion is valid as it simplifies the code by removing the unnecessary Boolean() conversion, which is redundant since `table.getIsSomePageRowsSelected()` already returns a boolean.
    8
    Enhancement
    Use nullish coalescing for more precise fallback handling ___ **Consider using nullish coalescing (??) instead of logical OR (||) for the fallback
    value. This will only use the fallback if data.name is null or undefined, not for
    other falsy values like an empty string.** [apps/platform/src/components/shared/navbar/index.tsx [41]](https://github.com/keyshade-xyz/keyshade/pull/413/files#diff-1463cdbabc4495c0e3893848022efb23ea972381c8d4de0f1ad206b210a0a6caR41-R41) ```diff -name: data.name?.split(' ')[0] ?? data.email.split('@')[0], +name: data.name?.split(' ')[0] ?? data.email?.split('@')[0] ?? '', ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why: The suggestion correctly identifies that using nullish coalescing (??) instead of logical OR (||) provides a more precise fallback handling, as it only considers null or undefined values, not other falsy values like an empty string.
    7
    unamdev0 commented 3 weeks ago

    @rajdip-b please note that other than optional chaining, I had to add checks here and here, cause after building, code was throwing reference error to localStorage , this was also in fact due to strict null check.

    Screenshot 2024-09-06 at 9 25 07 PM
    rajdip-b commented 3 weeks ago

    Yes, this was a known issue. Thanks for getting this solved!

    rajdip-b commented 2 weeks ago

    :tada: This PR is included in version 2.5.0 :tada:

    The release is available on GitHub release

    Your semantic-release bot :package::rocket: