feat: [AH-2815]: Added swift support for frontend#81
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds Swift package support across enums, types, factories, repository/version implementations, UI pages, styles, and localization; Swift entries are feature-flagged and some UI items are marked "Coming Soon." Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Factory
participant RepositoryStep
participant VersionStep
User->>UI: Navigate to create repository (choose Swift)
UI->>Factory: request repository step for SWIFT
Factory->>RepositoryStep: instantiate SwiftRepositoryType
RepositoryStep->>UI: render create form / default values
User->>UI: submit create request
UI->>RepositoryStep: validate & submit configuration
Note over UI,Factory: Later...
User->>UI: Open version details for Swift artifact
UI->>Factory: request version type for SWIFT
Factory->>VersionStep: instantiate SwiftVersionType
VersionStep->>UI: render version list / details / tabs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
web/src/ar/pages/version-details/SwiftVersion/styles.module.scss (1)
1-3:--page-header-height: 137pxis a magic number.If this value is shared across Swift views or tied to a measured element, extracting it into a named comment or a shared token would improve maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/ar/pages/version-details/SwiftVersion/styles.module.scss` around lines 1 - 3, The style uses a magic number in the .pageBody rule via --page-header-height: 137px;—replace this literal with a shared, named token (e.g., a global CSS variable or theme token) and update any other Swift views to consume that token so the value is centralized; modify the declaration in styles.module.scss to reference the shared variable name (and add a short comment showing what it represents) and add the token to the common theme or :root so measurements tied to a real element can be adjusted in one place and kept consistent across VersionDetails and other Swift views.web/src/ar/pages/version-details/SwiftVersion/pages/overview/styles.module.scss (1)
6-6:column-gapuses a hardcoded pixel value whilerow-gapuses a design token.- column-gap: 30px; + column-gap: var(--spacing-xlarge);Replace with the appropriate spacing token from the design system to stay consistent with
row-gap: var(--spacing-medium)on line 5.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/ar/pages/version-details/SwiftVersion/pages/overview/styles.module.scss` at line 6, The CSS uses a hardcoded column-gap (column-gap: 30px) next to row-gap: var(--spacing-medium); replace the hardcoded value by using the design token (e.g., column-gap: var(--spacing-medium)) in the styles.module.scss overview rules so both gaps use the same spacing token.web/src/ar/pages/version-details/SwiftVersion/pages/artifact-details/SwiftArtifactDetailsPage.tsx (1)
29-29: Consider addingtypemodifier toSwiftArtifactDetailsimport.
SwiftArtifactDetailsis used only as a type parameter (line 36) but is imported as a value. For consistency withtype SwiftVersionDetailsQueryParamsin the same import statement, and to be safe withisolatedModules, prefix it withtype.Suggested fix
-import { SwiftArtifactDetails, SwiftArtifactDetailsTabEnum, type SwiftVersionDetailsQueryParams } from '../../types' +import { type SwiftArtifactDetails, SwiftArtifactDetailsTabEnum, type SwiftVersionDetailsQueryParams } from '../../types'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/ar/pages/version-details/SwiftVersion/pages/artifact-details/SwiftArtifactDetailsPage.tsx` at line 29, The import currently brings SwiftArtifactDetails in as a value but it's only used as a type (e.g., as a generic/type parameter in SwiftArtifactDetailsTabEnum usage), which can break isolatedModules; change the import statement to import SwiftArtifactDetails with the type modifier (i.e., add "type" before SwiftArtifactDetails) while leaving SwiftArtifactDetailsTabEnum and SwiftVersionDetailsQueryParams as-is so the line reads similarly to the existing "type SwiftVersionDetailsQueryParams" import.web/src/ar/pages/version-details/SwiftVersion/SwiftVersionType.tsx (1)
52-52: Use the enum value directly instead of a string literal cast.
'SWIFT' as RepositoryPackageTypebypasses type safety. TheSwiftRepositoryTypeclass (inSwiftRepositoryType.tsx) already usesRepositoryPackageType.SWIFT— this should be consistent.Suggested fix
- protected packageType = 'SWIFT' as RepositoryPackageType + protected packageType = RepositoryPackageType.SWIFT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/ar/pages/version-details/SwiftVersion/SwiftVersionType.tsx` at line 52, Replace the string-cast assignment in SwiftVersionType by using the enum directly: change the protected member packageType currently set as "'SWIFT' as RepositoryPackageType" to use RepositoryPackageType.SWIFT (consistent with SwiftRepositoryType); update the declaration in SwiftVersionType.tsx so packageType is typed and initialized with the enum value instead of a string cast to preserve type safety.web/src/ar/pages/repository-details/SwiftRepository/SwiftRepositoryType.tsx (1)
51-76:defaultUpstreamProxyValuesis dead code whensupportsUpstreamProxy = false.Since
supportsUpstreamProxyisfalse, the framework will never readdefaultUpstreamProxyValues. The 13-line block can be removed entirely.♻️ Proposed cleanup
protected supportsUpstreamProxy = false protected isWebhookSupported = false - protected defaultUpstreamProxyValues: UpstreamRegistryRequest = { - packageType: RepositoryPackageType.SWIFT as UpstreamRegistryRequest['packageType'], - identifier: '', - config: { - type: RepositoryConfigType.UPSTREAM, - source: UpstreamRepositoryURLInputSource.Custom, - authType: UpstreamProxyAuthenticationMode.ANONYMOUS, - url: '' - }, - cleanupPolicy: [], - scanners: [], - isPublic: false - } - renderCreateForm(_props: CreateRepositoryFormProps): JSX.Element {This also removes the now-unused imports
UpstreamProxyAuthenticationMode,UpstreamRepositoryURLInputSource, andtype UpstreamRegistryRequest— verify no other usage exists in the file before removing them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/ar/pages/repository-details/SwiftRepository/SwiftRepositoryType.tsx` around lines 51 - 76, The block defaultUpstreamProxyValues is dead because supportsUpstreamProxy is false; remove the entire defaultUpstreamProxyValues declaration and any related unused imports (UpstreamProxyAuthenticationMode, UpstreamRepositoryURLInputSource, and the UpstreamRegistryRequest type) from SwiftRepositoryType.tsx, ensure supportsUpstreamProxy remains false, and run a quick grep/IDE search to confirm those symbols aren’t used elsewhere in the file before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/ar/pages/repository-details/SwiftRepository/SwiftRepositoryType.tsx`:
- Line 55: The file uses unsafe type assertions for Swift package type because
the external types lack SWIFT; once `@harnessio/react-har-service-client` adds
SWIFT to its unions, remove the three casts: replace
"RepositoryPackageType.SWIFT as VirtualRegistryRequest['packageType']" in the
Virtual registry assignment, "RepositoryPackageType.SWIFT as
UpstreamRegistryRequest['packageType']" in the Upstream registry assignment, and
"RepositoryPackageType.SWIFT as Parameters<typeof
SetupClientContent>[0]['packageType']" where packageType is passed to
SetupClientContent with plain "RepositoryPackageType.SWIFT" (no as-cast) and run
typecheck to ensure the updated external types accept it.
In
`@web/src/ar/pages/version-details/SwiftVersion/pages/overview/styles.module.scss`:
- Around line 8-21: Stylelint is flagging CSS Modules :global(...) as unknown
pseudo-classes (seen in selectors like
:global(.StyledProps--font-variation-small-bold),
:global(.StyledProps--font-variation-small), and
:global(.CopyToClipboard--copyIcon)); update the project Stylelint config
(.stylelintrc.json) to use a CSS‑Modules-aware parser/customSyntax (e.g.,
postcss-scss or `@stylelint/postcss-css-in-js` with CSS Modules support) or add a
plugin/override that recognizes :global() so these selectors are not reported,
and ensure the rule selector-pseudo-class-no-unknown is disabled or configured
for CSS Modules contexts; also replace the hardcoded column-gap: 30px with the
project spacing token (e.g., var(--spacing-medium)) to match row-gap usage for
consistent spacing.
In `@web/src/ar/pages/version-details/SwiftVersion/styles.module.scss`:
- Around line 5-10: The .cardContainer rule uses width: 60% !important together
with min-width: 1040px which forces a fixed minimum that overrides the percent
width on typical desktop viewports and causes horizontal overflow; update the
styling by moving this layout into a shared responsive rule (e.g., a shared
version-details/base component stylesheet) or adjust/remove the rigid min-width
to a responsive breakpoint-based value so width: 60% can apply, and remove the
unnecessary !important where possible; locate and change the .cardContainer
declaration in SwiftVersion/styles.module.scss and the same pattern across other
version-detail modules (Python, Ruby, Go, Cargo, Conda, Docker, Helm, Maven,
artifact-details) to use breakpoint media queries or a centralized class so
behavior is consistent and non-overflowing.
---
Nitpick comments:
In `@web/src/ar/pages/repository-details/SwiftRepository/SwiftRepositoryType.tsx`:
- Around line 51-76: The block defaultUpstreamProxyValues is dead because
supportsUpstreamProxy is false; remove the entire defaultUpstreamProxyValues
declaration and any related unused imports (UpstreamProxyAuthenticationMode,
UpstreamRepositoryURLInputSource, and the UpstreamRegistryRequest type) from
SwiftRepositoryType.tsx, ensure supportsUpstreamProxy remains false, and run a
quick grep/IDE search to confirm those symbols aren’t used elsewhere in the file
before committing.
In
`@web/src/ar/pages/version-details/SwiftVersion/pages/artifact-details/SwiftArtifactDetailsPage.tsx`:
- Line 29: The import currently brings SwiftArtifactDetails in as a value but
it's only used as a type (e.g., as a generic/type parameter in
SwiftArtifactDetailsTabEnum usage), which can break isolatedModules; change the
import statement to import SwiftArtifactDetails with the type modifier (i.e.,
add "type" before SwiftArtifactDetails) while leaving
SwiftArtifactDetailsTabEnum and SwiftVersionDetailsQueryParams as-is so the line
reads similarly to the existing "type SwiftVersionDetailsQueryParams" import.
In
`@web/src/ar/pages/version-details/SwiftVersion/pages/overview/styles.module.scss`:
- Line 6: The CSS uses a hardcoded column-gap (column-gap: 30px) next to
row-gap: var(--spacing-medium); replace the hardcoded value by using the design
token (e.g., column-gap: var(--spacing-medium)) in the styles.module.scss
overview rules so both gaps use the same spacing token.
In `@web/src/ar/pages/version-details/SwiftVersion/styles.module.scss`:
- Around line 1-3: The style uses a magic number in the .pageBody rule via
--page-header-height: 137px;—replace this literal with a shared, named token
(e.g., a global CSS variable or theme token) and update any other Swift views to
consume that token so the value is centralized; modify the declaration in
styles.module.scss to reference the shared variable name (and add a short
comment showing what it represents) and add the token to the common theme or
:root so measurements tied to a real element can be adjusted in one place and
kept consistent across VersionDetails and other Swift views.
In `@web/src/ar/pages/version-details/SwiftVersion/SwiftVersionType.tsx`:
- Line 52: Replace the string-cast assignment in SwiftVersionType by using the
enum directly: change the protected member packageType currently set as "'SWIFT'
as RepositoryPackageType" to use RepositoryPackageType.SWIFT (consistent with
SwiftRepositoryType); update the declaration in SwiftVersionType.tsx so
packageType is typed and initialized with the enum value instead of a string
cast to preserve type safety.
| & :global(.StyledProps--font-variation-small-bold) { | ||
| font-size: 13px !important; | ||
| } | ||
|
|
||
| & :global(.StyledProps--font-variation-small) { | ||
| font-size: 13px !important; | ||
| } | ||
|
|
||
| & :global(.CopyToClipboard--copyIcon) { | ||
| & > svg { | ||
| height: 16px; | ||
| width: 16px; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Stylelint configuration files
echo "=== Searching for Stylelint config files ==="
find . -maxdepth 3 -name ".stylelintrc*" -o -name "stylelint.config.*" 2>/dev/null | head -20
# Check for Stylelint in package.json or common config locations
echo "=== Checking package.json for Stylelint config ==="
if [ -f package.json ]; then
cat package.json | grep -A 20 '"stylelint"' || echo "No stylelint config in package.json"
fi
# Look for stylelint configuration in root
echo "=== Files in root directory ==="
ls -la | grep -i stylelint || echo "No stylelint files in root"Repository: abhinavcode/harness
Length of output: 280
🏁 Script executed:
#!/bin/bash
# Search for CSS Modules references in Stylelint config
echo "=== Searching for CSS Modules and postcss-modules references ==="
rg "postcss-modules|css-modules|customSyntax.*scss" --type=json --type=yaml --type=js 2>/dev/null | head -30
# Search for selector-pseudo-class-no-unknown rule configuration
echo "=== Searching for selector-pseudo-class-no-unknown configuration ==="
rg "selector-pseudo-class-no-unknown" 2>/dev/null | head -30
# Check for :global in .stylelintrc or similar
echo "=== Searching for :global or global pseudo-class handling ==="
rg '"global"|:global' --type=json --type=yaml --type=js -B2 -A2 2>/dev/null | head -40Repository: abhinavcode/harness
Length of output: 380
🏁 Script executed:
#!/bin/bash
# Check the actual SCSS file mentioned in the review
echo "=== Checking the file mentioned in review ==="
if [ -f "web/src/ar/pages/version-details/SwiftVersion/pages/overview/styles.module.scss" ]; then
echo "File exists. Content:"
cat -n "web/src/ar/pages/version-details/SwiftVersion/pages/overview/styles.module.scss" | head -30
else
echo "File does not exist. Searching for similar paths:"
find . -name "styles.module.scss" -path "*SwiftVersion*" 2>/dev/null
fiRepository: abhinavcode/harness
Length of output: 725
🏁 Script executed:
#!/bin/bash
# Read the complete .stylelintrc.json
echo "=== Full .stylelintrc.json content ==="
cat -n .stylelintrc.jsonRepository: abhinavcode/harness
Length of output: 1318
🏁 Script executed:
#!/bin/bash
# Check if there are any plugins or overrides for CSS Modules
echo "=== Checking for plugins in .stylelintrc.json ==="
jq '.plugins // []' .stylelintrc.json
echo "=== Checking for customSyntax in .stylelintrc.json ==="
jq '.customSyntax // "not set"' .stylelintrc.json
echo "=== Checking overrides for .module.scss ==="
jq '.overrides // []' .stylelintrc.json
echo "=== Checking selector-pseudo-class-no-unknown rule ==="
jq '.rules."selector-pseudo-class-no-unknown" // "not explicitly configured"' .stylelintrc.jsonRepository: abhinavcode/harness
Length of output: 310
Update Stylelint config to support CSS Modules :global() syntax.
The three selector-pseudo-class-no-unknown errors for :global(...) are valid CSS Modules syntax. The current .stylelintrc.json extends stylelint-config-standard-scss without CSS Modules support — it lacks customSyntax and any postcss-modules plugin configuration. Configure the Stylelint setup to handle CSS Modules (e.g., via postcss-scss with CSS Modules awareness or the appropriate parser) to suppress these false positives project-wide.
Additionally, column-gap: 30px is hardcoded while row-gap uses var(--spacing-medium) — consider using a consistent approach for spacing values.
🧰 Tools
🪛 Stylelint (17.3.0)
[error] 8-8: Unexpected unknown pseudo-class selector ":global" (selector-pseudo-class-no-unknown)
(selector-pseudo-class-no-unknown)
[error] 12-12: Unexpected unknown pseudo-class selector ":global" (selector-pseudo-class-no-unknown)
(selector-pseudo-class-no-unknown)
[error] 16-16: Unexpected unknown pseudo-class selector ":global" (selector-pseudo-class-no-unknown)
(selector-pseudo-class-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/src/ar/pages/version-details/SwiftVersion/pages/overview/styles.module.scss`
around lines 8 - 21, Stylelint is flagging CSS Modules :global(...) as unknown
pseudo-classes (seen in selectors like
:global(.StyledProps--font-variation-small-bold),
:global(.StyledProps--font-variation-small), and
:global(.CopyToClipboard--copyIcon)); update the project Stylelint config
(.stylelintrc.json) to use a CSS‑Modules-aware parser/customSyntax (e.g.,
postcss-scss or `@stylelint/postcss-css-in-js` with CSS Modules support) or add a
plugin/override that recognizes :global() so these selectors are not reported,
and ensure the rule selector-pseudo-class-no-unknown is disabled or configured
for CSS Modules contexts; also replace the hardcoded column-gap: 30px with the
project spacing token (e.g., var(--spacing-medium)) to match row-gap usage for
consistent spacing.
| .cardContainer { | ||
| width: 60% !important; | ||
| min-width: 1040px; | ||
| padding: var(--spacing-7) !important; | ||
| background-color: var(--white); | ||
| margin: var(--spacing-large); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for similar width + min-width pattern in SCSS files, especially in other package types
rg -n "min-width.*1040|width.*60%" --type scss -A2 -B2Repository: abhinavcode/harness
Length of output: 91
🏁 Script executed:
# Also search for other package-type version details pages to compare patterns
fd "version.*detail|SwiftVersion" -e scss | head -20Repository: abhinavcode/harness
Length of output: 45
🏁 Script executed:
# Get broader context: find all version-details pages
fd "version-details" -type dRepository: abhinavcode/harness
Length of output: 233
🏁 Script executed:
# Find all .scss files in the repository
fd -e scss | head -30Repository: abhinavcode/harness
Length of output: 1818
🏁 Script executed:
# Search for min-width pattern in all files
rg "min-width.*1040" -A2 -B2Repository: abhinavcode/harness
Length of output: 8834
🏁 Script executed:
# Search for the width 60% pattern
rg "width.*60%" -A2 -B2Repository: abhinavcode/harness
Length of output: 11754
🏁 Script executed:
# Check the structure of version-details directory
find . -path "*/version-details/*" -type f | head -20Repository: abhinavcode/harness
Length of output: 1805
min-width: 1040px overrides width: 60% on typical desktop viewports—this pattern is copy-pasted across 16+ version-detail pages.
The same width: 60% !important + min-width: 1040px pattern exists in all version-detail package implementations (Python, Ruby, Go, Cargo, Conda, Docker, Helm, Maven, etc.) and in artifact-details pages. On typical desktop resolutions (~1280–1440px viewport) with sidebar and page chrome, the effective parent width will be well under the ~1733px threshold needed for width: 60% to bind, making min-width: 1040px the active constraint and likely causing horizontal overflow.
Either centralize this layout constraint into a shared base stylesheet and make it responsive, or adjust min-width to fit the actual container width. Verify that the current behavior is intentional across all these pages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/ar/pages/version-details/SwiftVersion/styles.module.scss` around
lines 5 - 10, The .cardContainer rule uses width: 60% !important together with
min-width: 1040px which forces a fixed minimum that overrides the percent width
on typical desktop viewports and causes horizontal overflow; update the styling
by moving this layout into a shared responsive rule (e.g., a shared
version-details/base component stylesheet) or adjust/remove the rigid min-width
to a responsive breakpoint-based value so width: 60% can apply, and remove the
unnecessary !important where possible; locate and change the .cardContainer
declaration in SwiftVersion/styles.module.scss and the same pattern across other
version-detail modules (Python, Ruby, Go, Cargo, Conda, Docker, Helm, Maven,
artifact-details) to use breakpoint media queries or a centralized class so
behavior is consistent and non-overflowing.
Summary by CodeRabbit