Skip to content

fix: preserve baseURL query string when joining request path - #649

Open
EnesKartal wants to merge 1 commit into
unjs:mainfrom
EnesKartal:fix/baseurl-query-path-648
Open

EnesKartal wants to merge 1 commit into
unjs:mainfrom
EnesKartal:fix/baseurl-query-path-648

Conversation

@EnesKartal

@EnesKartal EnesKartal commented Nov 7, 2026 •

Copy link
Copy Markdown

Resolves #648

Summary

When baseURL contains a query string (e.g. http://localhost:3000/api?key=abc), passing a request path (e.g. /users) previously joined the path onto the full base URL string, resulting in /api?key=abc/users (putting /users into the query parameter value rather than joining it onto the URL pathname).

Solution

In withBase:

  • If base has no query string or hash, preserve the existing fast path.
  • When base contains query parameters (or hash fragments), decompose base and input into their respective path, query, and hash components.
  • Join the path components cleanly with joinURL.
  • Merge query parameters (preserving base query parameters while allowing request query parameters to take precedence).
  • Preserve hash fragments.

Tests

  • Added integration tests in test/index.test.ts for $fetch with baseURL containing query string (with and without additional query options).
  • Added comprehensive unit tests for withBase covering simple paths, query strings in base, query strings in both base and input, and hash fragments.

Summary by CodeRabbit

  • Bug Fixes
    • Improved URL handling when a base URL includes query parameters or a fragment. Base and input paths are joined without duplicating the base path, and fragments are preserved with the input fragment taking precedence.
    • When both URLs include query parameters, matching base values are replaced by input values while repeated input parameters are retained. Queries are also preserved when only one URL includes them.

@coderabbitai

coderabbitai Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Explain Change →

Walkthrough

withBase now joins URL paths separately from query strings and fragments when the base contains a query or fragment. Input query values replace matching base values, and an input fragment takes precedence. New tests cover withBase and $fetch behavior.

Changes: Base URL composition

Layer / File(s) Summary
URL joining and validation
src/utils.url.ts, test/index.test.ts
withBase separates paths, queries, and fragments when the base contains a query or fragment. Tests cover path joining, query precedence, fragment handling, and $fetch requests with base URL queries.

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: johannschopplich

Merge Risk: Minimal · up to b78a1

No actionable merge-blocking risk introduced by this change is established. The existing URL edge cases can be addressed separately.

Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title states the main change: withBase now keeps the baseURL query string when it joins the request path. The raw summary and objectives confirm that src/utils.url.ts changes withBase to split pat…
Linked Issues check Passed [#648] The issue requires that a request path join the pathname of a query-bearing baseURL, not the query value. The reviewed head changes withBase in src/utils.url.ts. When base contains '?' or '#', …
Out of Scope Changes check Passed The whole-PR diff (merge base 1dbc37f to head b78a1ed) changes src/utils.url.ts and test/index.test.ts. The src/utils.url.ts changes are limited to withBase. They handle the query and hash parts…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check

Explanation

The title states the main change: withBase now keeps the baseURL query string when it joins the request path. The raw summary and objectives confirm that src/utils.url.ts changes withBase to split path, query, and fragment, and that test/index.test.ts adds $fetch and withBase tests for query-bearing baseURLs. The title is specific and matches the linked issue #648.

Full details: Linked Issues check

Explanation

[#648] The issue requires that a request path join the pathname of a query-bearing baseURL, not the query value. The reviewed head changes withBase in src/utils.url.ts. When base contains '?' or '#', withBase splits base and input into path, query, and hash. It joins the paths with joinURL and keeps the base query. Tracing the issue examples through the code: withBase('/users', 'http://localhost:3000/api?key=abc') returns 'http://localhost:3000/api/users?key=abc'. src/fetch.ts calls withBase before withQuery. With query { page: 2 }, withQuery appends page=2, giving 'http://localhost:3000/api/users?key=abc&page=2'. Both match the expected output in the issue. The fast path for base values without '?' or '#' is unchanged. The test file adds $fetch and withBase tests for this case. I did not run the tests. The issue does not impose a parameter-precedence rule. The precedence behavior (input values replace base values) is a design choice in this PR and is not checked against the issue.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff (merge base 1dbc37f to head b78a1ed) changes src/utils.url.ts and test/index.test.ts. The src/utils.url.ts changes are limited to withBase. They handle the query and hash parts of base and input, which is the behavior the issue describes. The query merge logic (input keys replace base keys on first occurrence, repeated input keys are appended) and the hash precedence (input hash first, base hash otherwise) support the same query and hash handling. They do not alter withQuery, joinURL, or other exported functions. The test file changes cover withBase and $fetch behavior for this case. No unrelated files are changed. Parameter precedence and hash precedence are PR-specific choices. They are reasonable extensions of the same code path and are not unrelated changes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Additional comments: 1

src/utils.url.ts-80-83: 1

80-83: Minor · Functional Correctness · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

The raw startsWith check can skip the base path join on a partial segment match.

The check at Line 81 has no segment-boundary test. Take base = /api?key=abc and input = /apiary/users. _basePath is /api, and /apiary/users starts with it. So withBase returns /apiary/users?key=abc and does not join. The expected result is /api/apiary/users?key=abc.

The old no-query path had the same flaw, so this is not a new regression. The new branch copies it. The new tests do not cover it.

Also, when basePath is empty, _basePath becomes /. This happens for a base such as ?key=abc. Then any input that starts with / matches and is never joined. That is probably acceptable here. Still, keep a boundary check in mind if you fix this.

Proposed boundary check

-  const joinedPath = inputPath.startsWith(_basePath)
+  const hasBasePrefix =
+    inputPath === _basePath ||
+    inputPath.startsWith(_basePath === "/" ? "/" : `${_basePath}/`);
+  const joinedPath = hasBasePrefix
     ? inputPath
     : joinURL(_basePath, inputPath);
Review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 93229905-b514-438c-a7e2-b77eea5d7aab
Commits

Reviewing files that changed from the base of the PR and between 1dbc37f and b78a1ed.

Files selected for processing: 2

  • src/utils.url.ts
  • test/index.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

baseURL with a query string puts the request path inside the query

1 participant