fix(fetch): restore urlEncodeParameters support for the fetch client (#3343) - #3405
Conversation
Move makeRouteSafe and wrapRouteParameters from @orval/query/utils into @orval/core/getters/route so non-query packages can reuse them without introducing a reverse dependency on @orval/query. Update query/src/client.ts to import makeRouteSafe from @orval/core and move the related unit tests to core/getters/route.test.ts. Behavior is unchanged. This prepares for the follow-up fix that restores urlEncodeParameters support in @orval/fetch (orval-labs#3343). Signed-off-by: zeriong <jaeryong95@gmail.com>
The fetch client generator stopped honoring the `urlEncodeParameters` output option after commit cb06139 (orval-labs#2168) accidentally dropped the guard introduced by orval-labs#2292. Generated URL helpers interpolated path parameters as-is, so values containing reserved characters (`/`, `#`, `?`, ...) leaked into the request URL. Re-apply `makeRouteSafe(route)` at the top of `generateRequestFunction` in `@orval/fetch`, mirroring the axios path in `@orval/query`. This wraps each `\${param}` segment with `encodeURIComponent(String(...))`. Query parameters remain handled by `URLSearchParams` as before; this option only affects path parameters. Add a dedicated fetch + `urlEncodeParameters` snapshot fixture so a future regression in the fetch path won't be silently absorbed by the `dateParams` fixture.
State explicitly that the option wraps only path parameters (query parameters are already encoded by the underlying client) and note that array/object path parameters fall back to `String(value)` instead of following their OpenAPI `style` serialization, since orval does not generate `style: simple|matrix|label` for path parameters.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds core helpers to wrap template path parameters with encodeURIComponent(String(...)), applies them conditionally in the fetch generator when ChangesURL Parameter Encoding
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
…t drift The model files generated by this fixture exhibited a JSDoc reflow only on CI (Ubuntu) that didn't reproduce locally, while the same petstore spec under other fixtures stayed stable. Drop `schemas` and `mock` from the fixture so it only emits `endpoints.ts`; that single file still exercises the regression covered by orval-labs#3343 — `${encodeURIComponent( String(petId))}` is asserted in the URL helper output.
After the master merge of orval-labs#3393 (`fix(core): handle multi-line descriptions in JSDoc generation`), the core now emits multi-line JSDoc with proper ` * ` line prefixes. Regenerate the snapshot for this PR's new fixture so it matches the post-orval-labs#3393 output. The `encodeURIComponent(String(petId))` assertions covering orval-labs#3343 are preserved.
Summary
Closes #3343.
The fetch client generator stopped honoring the
urlEncodeParametersoutput option after commit cb06139 (#2168) accidentally dropped the guard introduced by #2292. As a result, generatedget<Op>Urlhelpers interpolated path parameters into the URL template literal as-is, so values containing reserved characters (/,#,?, ...) leaked into the request URL. The axios path in@orval/querywas unaffected because it has its ownmakeRouteSafecall site.This PR re-applies
makeRouteSafe(route)at the top ofgenerateRequestFunctionin@orval/fetch, mirroring the axios path. Each${param}segment in the generated URL is wrapped withencodeURIComponent(String(...))whenurlEncodeParameters: true. Query parameters remain handled byURLSearchParams(no double encoding); this option only affects path parameters.The fix is split across three commits to keep review surgical:
refactor(core,query)— relocatemakeRouteSafeandwrapRouteParametersfrom@orval/query/utilsto@orval/core/getters/routeso@orval/fetchcan reuse them without introducing a reverse dependency on@orval/query. Unit tests move alongside. No behavior change.fix(fetch)— apply the guard, and add a dedicatedfetch + urlEncodeParameterssnapshot fixture so a future regression in the fetch path won't be silently absorbed by the existingdateParamsfixture.docs(output)— clarify that the option only wraps path parameters and that array/object path params fall back toString(value)(orval does not generatestyle: simple|matrix|labelfor path parameters).Scope notes
pathRoute(used for OpenAPI spec lookup, e.g. MSW handlers) is intentionally left unencoded — it is a literal OpenAPI path string used as a key, not a runtime URL.@orval/queryhas the same gap but is out of scope for this issue (urlEncodeParameters option not working ? #3343 reports the fetch case); it can be tracked separately if desired.styleserialization (OpenAPIstyle: simple|matrix|label+explode) is unimplemented before and after this PR. Documented as a known limitation.Test plan
bun run buildpassesbun run lintpassesbun run typecheckpassesbun vitest run— only pre-existingresolve-version.test.tsfailures unrelated to this PR (verified by stashing the diff and re-running)bun run test:snapshotspasses; snapshot diffs are limited to:tests/__snapshots__/fetch/url-encode-parameters/(new fixture, exercises the fix)tests/__snapshots__/fetch/dateParams/pets/pets.ts(existing fetch fixture now correctly emitsencodeURIComponent(String(...)))tests/__snapshots__/vue-query/url-encode-parameters/endpoints.tsandtests/__snapshots__/vue-query/combination-used-by-maxim-mazurok/endpoints.ts(vue-query defaults to the fetch HTTP client; same fix flows through)Summary by CodeRabbit
New Features
Documentation
Tests