Fix scope path to end with slash

[chrisvire]

Summary by CodeRabbit

• Bug Fixes
  • Ensure consistent trailing-slash normalization for the build base path used by the web manifest (start_url and scope) and the service worker scope.
  • Use a normalized build base path that falls back to "/" when unset and preserves/appends a trailing slash when set.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR centralizes and normalizes BUILD_BASE_PATH handling: adds trailing-slash normalization in svelte.config.js for service worker scope and uses the same normalized base path in convert/convertManifest.ts for manifest start_url and scope.

Changes

Cohort / File(s)	Summary
Service worker scope normalization svelte.config.js	Adds serviceWorkerScope helper that normalizes process.env.BUILD_BASE_PATH to include a trailing slash (or defaults to /) and replaces direct env usage for service worker scope.
Manifest start_url / scope normalization convert/convertManifest.ts	Introduces normalizePath and derives pwaPath from BUILD_BASE_PATH (normalized) or '/'; start_url and scope now use this normalized pwaPath. Existing icon handling and manifest-file branching remain unchanged.

Sequence Diagram(s)

(omitted — changes are small normalization adjustments without multi-component flow requiring visualization)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Add scope to service worker configuration #925: Adds service worker scope using BUILD_BASE_PATH in svelte.config.js; closely related to this PR's scope normalization changes.

Poem

🐇 I hopped along the build-time track,

I added slashes, never looked back,
Now scopes and starts align so neat,
Manifests march to a single beat,
Hooray — the paths are tidy and track!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the primary change—ensuring scope paths end with a trailing slash, which is the core modification across both modified files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

convert/convertManifest.ts (1)

31-38: Confirm default "/" is intended when BUILD_BASE_PATH is unset.

This now forces start_url and scope to "/" whenever the env var is absent; please verify that’s the desired behavior for apps that might have relied on a relative ./ or an existing manifest value. Also, consider hoisting pwaPath outside the loop to avoid re‑computing it for every line.

♻️ Optional refactor: compute once

-        contents = lines
-            .map((line) => {
-                const pwaPath = process.env.BUILD_BASE_PATH
-                    ? normalizePath(process.env.BUILD_BASE_PATH)
-                    : '/';
+        const pwaPath = process.env.BUILD_BASE_PATH
+            ? normalizePath(process.env.BUILD_BASE_PATH)
+            : '/';
+        contents = lines
+            .map((line) => {
                 if (line.includes('start_url')) {
                     line = `  "start_url" : "${pwaPath}",`;
                 }
                 if (line.includes('scope')) {
                     line = `  "scope" : "${pwaPath}",`;
                 }

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc18a4f and 8d5878c.

📒 Files selected for processing (1)

• convert/convertManifest.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: test
• GitHub Check: lint

🔇 Additional comments (1)

convert/convertManifest.ts (1)

10-16: normalizePath helper looks good.

Clear, single‑responsibility helper with the expected trailing‑slash behavior.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@convert/convertManifest.ts`:
- Around line 27-34: The local variable named `path` inside the scope-handling
block shadows the imported `path` module and breaks calls like
`path.join(dataDir, bareFileName)`; rename that local variable (for example to
`scopePath` or `buildBasePath`) in the `if (line.includes('scope')) { ... }`
block and update its usage in the template literal (`"scope" : "${...}",`) so
the imported `path` module remains available for calls such as `path.join(...)`.

🧹 Nitpick comments (1)

convert/convertManifest.ts (1)

23-26: Note: Pre-existing variable shadowing issue here as well.

Line 24 also declares a const path that shadows the imported path module. While this isn't part of your changes, consider renaming both to basePath or similar for consistency and to resolve the shadowing throughout this function.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d4fa563 and a916c1f.

📒 Files selected for processing (2)

• convert/convertManifest.ts
• svelte.config.js

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: test
• GitHub Check: lint

🔇 Additional comments (2)

svelte.config.js (2)

10-16: LGTM! Clean implementation of scope normalization.

The helper variable with clear comments effectively normalizes the trailing slash. Consider extracting this logic into a shared utility if it needs to be reused elsewhere (e.g., convertManifest.ts uses identical logic).

---

43-47: LGTM!

The service worker scope now correctly uses the normalized path with a trailing slash.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@convert/convertManifest.ts`:
- Around line 23-26: The start_url handling can produce a double slash when
BUILD_BASE_PATH ends with '/', so update the logic in the block that checks
line.includes('start_url') to normalize urlPath like scopePath does: compute
urlPath from process.env.BUILD_BASE_PATH (falling back to '.') and ensure it
does not end with a trailing slash before appending a single '/' when building
the line string for "start_url" (refer to the urlPath variable and the code that
sets line = `  "start_url" : "${urlPath}/",`). Ensure the same trailing-slash
check used for scopePath is applied to urlPath to avoid producing '//'.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a916c1f and dc18a4f.

📒 Files selected for processing (2)

• convert/convertManifest.ts
• svelte.config.js

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: test
• GitHub Check: lint

🔇 Additional comments (3)

svelte.config.js (2)

10-16: LGTM! Clear trailing-slash normalization for service worker scope.

The logic correctly handles all cases: appending a slash when BUILD_BASE_PATH is defined but lacks one, preserving it when already present, and defaulting to '/' when undefined. The comments provide good documentation of the intent.

---

43-46: LGTM!

Using the pre-computed serviceWorkerScope variable keeps the config object clean and consistent.

convert/convertManifest.ts (1)

27-34: LGTM! Trailing-slash normalization correctly applied.

The scopePath logic properly handles all cases and is consistent with the serviceWorkerScope implementation in svelte.config.js. The previous variable shadowing issue has been addressed.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}