Skip to content

Fix missing default Accept header when options is an empty object - #178

Open
soyuka wants to merge 1 commit into
mainfrom
fix/hydra-default-accept-header
Open

Fix missing default Accept header when options is an empty object#178
soyuka wants to merge 1 commit into
mainfrom
fix/hydra-default-accept-header

Conversation

@soyuka

Copy link
Copy Markdown
Member

Summary

Alternative fix for #177.

setHeaders() short-circuited on falsy options.headers, returning a plain {} instead of building Headers with the Accept default. This breaks @api-platform/admin's schemaAnalyzer.resolveSchemaParameters(), which always calls getParameters() with an explicit {}.

#177's fix (options.headers = {} instead of return) works, but mutates the caller-supplied options object as a side effect before falling through. This PR removes the early-return branch entirely and derives headers without mutating the input:

letheaders=typeofoptions.headers==="function"
? options.headers()
: (options.headers??{});
  • No mutation of the caller's options object.
  • Added a regression test asserting the default Accept header is sent when options is {}.
  • Added a test asserting fetchJsonLd does not mutate the passed options object.

Test plan

  • pnpm typecheck
  • pnpm format:check
  • pnpm test src/hydra/fetchJsonLd.test.ts (blocked locally by unrelated stray Yarn PnP files in this checkout; relying on CI)

setHeaders() short-circuited on falsy options.headers, returning a
plain {} instead of building Headers with the Accept default. This
broke @api-platform/admin's schemaAnalyzer, which always calls
getParameters() with an explicit {} (see #177).
Removed the early return so the function always falls through to the
Headers/Accept logic, and swapped the headers derivation to avoid
mutating the caller's options object as a side effect.
@github-actions

Copy link
Copy Markdown

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines71.18% (🎯 70%)294 / 413
🔵Statements71.15% (🎯 70%)296 / 416
🔵Functions70.31% (🎯 70%)45 / 64
🔵Branches60.1% (🎯 58%)238 / 396
File Coverage
FileStmtsBranchesFunctionsLinesUncovered Lines
Changed Files
src/hydra/fetchJsonLd.ts100%91.66%100%100%
Generated in workflow #319 for commit 468a076 by the Vitest Coverage Report Action

Sign up for freeto 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.

1 participant

@soyuka