openedx / openedx/frontend-app-learning

Remove single-use `joi` dependency from map-search-response in favor of a TypeScript-native approach

Open
#2,050 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
70
Forks
335
Avg merge
1d 17h
Merged PRs (30d)
35

Description

[!NOTE]
This issue was investigated and written by Claude Code, at the request of a maintainer.

Summary

joi is a runtime schema-validation dependency that is used in exactly one place in the codebase: src/course-home/courseware-search/map-search-response.js, to validate the courseware-search endpoint response before mapping it. It is the repository's only runtime validator (no zod/yup/superstruct/ajv), so it exists solely to support this one function. This issue proposes removing the dependency in favor of a TypeScript-native approach.

Findings

joi is a single-use dependency
  • Declared in package.json ("joi": "^17.11.0").
  • Imported once, at src/course-home/courseware-search/map-search-response.js:1 (const Joi = require('joi');).
  • Consumer chain (already React Query based):
    useCoursewareSearchResults (src/course-home/courseware-search/data/apiHooks.ts) → mapSearchResponse(data, keyword)endpointSchema.validate(response).
The schema validates far less than it appears to

endpointSchema declares each results[] item with top-level fields id / contentType / location / url / content, but the real payload nests all of that under result.data.*. In the fixture (test-data/mocked-response.json) a result item's keys are _index, _type, _id, data, score — so the fields the mapper actually reads (result.data.content.displayName, result.data.contentType, result.score, …) live under the data key, not at the item's top level.

Combined with:

  • every item field being optional (no .required()), and
  • .unknown(true) on the item object,

…the item-level schema never actually validates the content shape — those fields simply pass as unknown keys. The effective runtime guarantees are only:

  • took is a required number,
  • total is a required number,
  • maxScore is a number or null,
  • results is an array of objects.

The single "wrong format" test (mapSearchResponse({ foo: 'bar' }) throws) passes purely because required took/total are missing — none of the content-shape rules are exercised.

(Casing is not the issue: searchCourseContentFromAPI camelCases via camelCaseObject, so the top-level keys line up. The mismatch is the nesting level.)

Options

Three approaches seem reasonable; this issue intentionally does not pick one.

Option A — hand-written type guard (no new dependency)

Convert map-search-response.js to TypeScript, define SearchResponse interfaces, and add a small guard that checks took/total are numbers and results is an array, throwing NonRetryableError otherwise, returning a typed value. Removes joi and adds nothing. Matches the small amount of validation that is actually effective today.

Option B — swap joizod

Replace the schema with a TS-native validator whose types are inferred from the schema. Trades one runtime dependency for another; most compelling if the intent is genuinely thorough runtime validation (which would also mean fixing the nesting-level mismatch so item content is really validated).

Option C — type and cast, no runtime validation

Define the response interface and cast, dropping runtime validation entirely. Smallest change; removes joi; loses the throw-on-malformed-response guarantee (a downstream error would surface via React Query instead). One test would need to change.

Decisions to resolve

  1. Which option (A / B / C).
  2. Preserve the current loose behavior, or fix the dead item-level validation? A faithful port keeps validation to took/total/results; a "fix it properly" port validates result.data.* for real. These are different-sized changes.

Scope notes

  • The file currently uses CommonJS require(); converting it to TypeScript aligns it with the surrounding code.
  • Existing tests to keep green: src/course-home/courseware-search/map-search-response.test.js and the mapSearchResponse mock usage in src/course-home/courseware-search/data/apiHooks.test.tsx.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with package.json and src/course-home/courseware-search/map-search-response.js, then read map-search-response.test.js and the mapSearchResponse usage in data/apiHooks.test.tsx. Resolve with a maintainer whether to use option A, B, or C and whether to preserve or fix item-level validation. Done means joi is removed and the named tests remain green.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, typescript
Domain
frontend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.