Skip to content

refactor(cli-hooks): use optional chaining check to gather project dependencies#2548

Merged
zimeg merged 3 commits intoslackapi:mainfrom
KirobotDev:main
Apr 13, 2026
Merged

refactor(cli-hooks): use optional chaining check to gather project dependencies#2548
zimeg merged 3 commits intoslackapi:mainfrom
KirobotDev:main

Conversation

@KirobotDev
Copy link
Copy Markdown
Contributor

Summary

This PR applies Biome lint fixes to normalize code style across the monorepo and improve code readability.

Changes

  • style: Normalize line endings from CRLF to LF across 634 files
  • refactor(cli-hooks): Simplify optional chaining check in getProjectPackageVersion()

Files Modified

Details

Line Endings Normalization

Configured Biome formatter (lineEnding: "lf") to enforce consistent Unix-style line endings across all files.
This improves cross-platform compatibility and avoids unnecessary diffs between environments.

Optional Chaining Refactor

Simplified conditional check using optional chaining:

// Before
if (!currentVersionOutput.dependencies || !currentVersionOutput.dependencies[packageName])

// After  
if (!currentVersionOutput.dependencies?.[packageName])

Impact

  • No functional changes
  • Improves code consistency and readability
  • Reduces noise in future diffs

Testing

  • No runtime logic changes
  • Existing tests should pass without modification

Notes

  • Large diff due to line ending normalization
  • Recommended to review commit-by-commit or ignore whitespace changes

@KirobotDev KirobotDev requested a review from a team as a code owner April 12, 2026 19:54
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 12, 2026

🦋 Changeset detected

Latest commit: c96838e

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

This PR includes changesets to release 1 package
Name Type
@slack/cli-hooks 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

Copy link
Copy Markdown
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 Hey @KirobotDev! Thanks for sharing this refactor 🙏

I'm closing this PR for now since it's not addressing an issue in current implementations but do let me know if issues are appearing due to package setups otherwise! 🎁

async function getProjectPackageVersion(packageName) {
const stdout = await execWrapper(`npm list ${packageName} --depth=0 --json`);
const currentVersionOutput = JSON.parse(stdout);
if (!currentVersionOutput.dependencies || !currentVersionOutput.dependencies[packageName]) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👁️‍🗨️ thought: IMHO the chained pattern is confusing in this statement and might make later changes more difficult.

@zimeg zimeg closed this Apr 12, 2026
@zimeg
Copy link
Copy Markdown
Member

zimeg commented Apr 13, 2026

🔭 I'm finding now the latest linter settings might be catching the change earlier of this PR - let's reopen this!

  ! Change to an optional chain.
  
    192 │   const stdout = await execWrapper(`npm list ${packageName} --depth=0 --json`);
    193 │   const currentVersionOutput = JSON.parse(stdout);
  > 194 │   if (!currentVersionOutput.dependencies || !currentVersionOutput.dependencies[packageName]) {
        │       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    195 │     throw new Error(`Failed to gather project information about ${packageName}`);
    196 │   }
  
  i Unsafe fix: Change to an optional chain.
  
    192 192 │     const stdout = await execWrapper(`npm list ${packageName} --depth=0 --json`);
    193 193 │     const currentVersionOutput = JSON.parse(stdout);
    194     │ - ··if·(!currentVersionOutput.dependencies·||·!currentVersionOutput.dependencies[packageName])·{
        194 │ + ··if·(!currentVersionOutput.dependencies?.[packageName])·{
    195 195 │       throw new Error(`Failed to gather project information about ${packageName}`);
    196 196 │     }

@zimeg zimeg reopened this Apr 13, 2026
@zimeg zimeg added semver:patch area:performance issues where performance is a meaningful concern pkg:cli-hooks applies to `@slack/cli-hooks` labels Apr 13, 2026
@zimeg zimeg modified the milestones: cli-hooks@1.x, cli-hooks@next Apr 13, 2026
@zimeg zimeg changed the title fix: normalize line endings to LF and apply Biome lint fixes refactor(cli-hooks): use optional chaining check to gather project dependencies Apr 13, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.42%. Comparing base (b56deda) to head (c96838e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2548   +/-   ##
=======================================
  Coverage   87.42%   87.42%           
=======================================
  Files          62       62           
  Lines       10249    10249           
  Branches      415      415           
=======================================
  Hits         8960     8960           
  Misses       1268     1268           
  Partials       21       21           
Flag Coverage Δ
cli-hooks 87.42% <100.00%> (ø)
cli-test 87.42% <100.00%> (ø)
logger 87.42% <100.00%> (ø)
oauth 87.42% <100.00%> (ø)
socket-mode 87.42% <100.00%> (ø)
web-api 87.42% <100.00%> (ø)
webhook 87.42% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KirobotDev Thanks for sharing these fixes with us! 🎁

I'm unsure that the line feeds are different in this PR so let's merge this with the fix to formatting preferences called out!

@zimeg zimeg merged commit 1a6c510 into slackapi:main Apr 13, 2026
12 checks passed
@KirobotDev
Copy link
Copy Markdown
Contributor Author

Thanks for merge my pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:performance issues where performance is a meaningful concern cla:signed pkg:cli-hooks applies to `@slack/cli-hooks` semver:patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants