Auto-transform the Bible HTML from getPassage so the consumer doesn't have extra steps by cameronapak · Pull Request #216 · youversion/platform-sdk-react

cameronapak · 2026-04-20T19:02:06Z

Summary

Auto-transforms Bible HTML inside getPassage so consumers never need to call transformBibleHtml manually. Uses native DOMParser in browser, dynamic import('linkedom') on server. Added data-yv-transformed idempotency marker so double-transforms are a no-op.

5 files changed across core and ui: bible.ts (getHtmlAdapters + transform in getPassage), bible-html-transformer.ts (idempotency guard), bible-html-transformer.test.ts (3 idempotency tests), bible.test.ts (updated assertions for transformed output), verse.tsx (kept transform as XSS safety net for direct callers).

Verse.Html retains its transformBibleHtml call as defense-in-depth — the idempotency marker makes it a no-op for HTML that already went through getPassage.

All 609 tests pass (290 core, 258 hooks, 61 ui). Build, typecheck, lint green.

Context: Why transformBibleHtml Exists — And Where It May Not Be Needed

Test plan

Verify getPassage with format: 'html' returns transformed content (data-yv-transformed present)
Verify getPassage with format: 'text' returns raw content (no transformation)
Verify double-transform produces identical output (idempotency)
Verify footnotes are extracted into data-verse-footnote attributes
Verify Verse.Html still sanitizes raw HTML passed directly (XSS protection)

🤖 Generated with Claude Code

Greptile Summary

This PR auto-applies the Bible HTML transformation pipeline inside getPassage so consumers no longer need to call transformBibleHtml manually. It also migrates the server-side DOM library from linkedom to jsdom, adds a data-yv-transformed idempotency marker so double-transforms are a safe no-op, and introduces a transform: false opt-out for callers that want raw HTML or want to avoid the jsdom dependency.

getPassage behavior change: HTML-format responses are now automatically sanitized, verse-wrapped, and footnote-extracted; consumers can opt out with transform: false (6th positional arg). The Verse.Html component retains its own transformBibleHtml call as defense-in-depth, which becomes a no-op on already-transformed HTML thanks to the idempotency marker.
Idempotency guard order: sanitizeBibleHtmlDocument runs unconditionally before the data-yv-transformed early return, so XSS sanitization is never skipped even when the guard fires.
Dependency swap: linkedom removed; jsdom added as an optional peer dep with a descriptive error thrown when it is absent on the server, and @types/jsdom@28.0.1 added as a devDep — though this type package is 4 major versions ahead of the pinned jsdom@24.0.0 runtime.

Confidence Score: 4/5

Safe to merge with one packaging fix: the @types/jsdom version should be aligned to the pinned jsdom@24 runtime before publishing.

The auto-transform logic is well-structured — sanitization runs before the idempotency guard, the server path has a clear error message, and the opt-out parameter works correctly. The one concrete defect is @types/jsdom@28.0.1 being used as the type source while jsdom@24.0.0 is the pinned dev runtime and peer dep constraint. If any API surface visible to TypeScript differs between jsdom 24 and 28, compile-time checking passes but consumers on jsdom 24 could see runtime failures.

packages/core/package.json — the @types/jsdom devDependency version should match the pinned jsdom@24.0.0 runtime.

Important Files Changed

Filename	Overview
packages/core/src/bible.ts	Adds getHtmlAdapters() (browser DOMParser / server jsdom with try/catch) and wires auto-transform into getPassage with an opt-out transform?: boolean parameter; breaking behavior change for html-format callers is intentional and documented
packages/core/src/bible-html-transformer.ts	Adds TRANSFORMED_ATTR constant and idempotency guard; sanitization runs unconditionally before the early return, correctly addressing prior XSS concern; structural transforms and marker-setting are clean
packages/core/src/bible-html-transformer-server.ts	Migrated from linkedom static import to jsdom static import; wrapper pattern unchanged, jsdom constructor usage is basic and stable
packages/core/package.json	Swaps linkedom peer dep for jsdom; @types/jsdom@^28.0.1 devDep is 4 major versions ahead of the pinned jsdom@24.0.0 devDep, creating a type/runtime mismatch
packages/ui/src/components/verse.tsx	Comment-only update; behavior unchanged — DOMParser guard and idempotent transform call retained as intended XSS safety net
packages/core/src/tests/bible.test.ts	Tests updated to assert transformed output and new transform: false opt-out; all tests exercise the DOMParser path (vitest provides DOMParser), so the server-side jsdom import branch in getHtmlAdapters is not covered here
packages/core/src/bible-html-transformer.test.ts	Adds three idempotency tests covering marker presence, short-circuit behaviour, and output equality on double-transform; well-structured
packages/core/src/styles/bible-reader.css	Adds :not(:has([data-yv-transformed])) .yv-vlbl::after rule to inject non-breaking space for untransformed HTML; modern :has() selector, no issues

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/core/package.json:50
`@types/jsdom@^28.0.1` is 4 major versions ahead of the pinned runtime `"jsdom": "24.0.0"` devDependency. TypeScript validates the code against jsdom 28 type definitions while the actual test/dev environment runs jsdom 24. If any jsdom 28 API used here was added, removed, or changed between versions 24 and 28, compile-time checking would pass while the code silently breaks at runtime. The type package should match the pinned runtime version.

```suggestion
    "@types/jsdom": "^24.0.0",
```

### Issue 2 of 2
packages/core/src/__tests__/bible.test.ts:124-136
**Server-side `getHtmlAdapters` path not covered by `bible.test.ts`**

All tests in this file run inside vitest's jsdom environment, so `globalThis.DOMParser` is always defined. This means every `getPassage` call exercises the `DOMParser` branch of `getHtmlAdapters()` — the `await import('jsdom')` server path is never hit here. If the server branch regresses (e.g., a wrong property path on `jsdom.JSDOM`), these tests will still pass. The existing `bible-html-transformer.server.test.ts` covers the transformer logic through jsdom, but the integration path in `getHtmlAdapters()` itself (the `try/catch` import and the adapter closure) has no test coverage.

_{Reviews (7): Last reviewed commit: "Merge branch 'main' into transform-bible..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

getPassage now automatically sanitizes and transforms HTML content before returning — verse wrapping, footnote extraction, nbsp, and table fixes all happen at the root. Uses native DOMParser in browser, dynamic import('linkedom') on server. Added data-yv-transformed idempotency marker so double-transforms are a no-op. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

changeset-bot · 2026-04-20T19:02:10Z

🦋 Changeset detected

Latest commit: 36e2adb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@youversion/platform-core	Minor
@youversion/platform-react-hooks	Minor
@youversion/platform-react-ui	Minor
vite-react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Run XSS sanitization before idempotency check so data-yv-transformed cannot bypass sanitizeBibleHtmlDocument. Add clear error message when linkedom is missing on server instead of opaque module-not-found error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

cameronapak · 2026-04-20T19:28:23Z


    expect(result.html).not.toContain('onclick');
-    expect(result.html).toContain('<p>');
+    expect(result.html).toContain('<p');


note: if you're wondering why this tag is seemingly cut off, it's because the tags would contain a data attribute in this new PR, which would then make it where the tag is something like <p data-yv-attribute> versus <p> standalone

can this expect() call do regular expressions? we don't have pre tags yet, that I know of, but still... it'd be great to tighten this up if that's not too difficult.

davidfedor

Big picture: I love the idea of being helpful, without requiring the developer to have to make another call. My comments and questions are around whether this is the best way to do that. (Maybe it is! I'm not sure yet.)
I notice this would be blurring the lines between Core being merely an API helper-layer, but now it would be doing some of the prep-work of the UI (visualization layer). So at the least having that be optional seems wise.
I'm wondering if that parameter should default to do the transformation, or not... or whether we need to force the dev to make a choice (to attempt to force them to make an informed choice).

davidfedor · 2026-04-23T19:15:45Z

+  } catch {
+    throw new Error(
+      'Server-side HTML transformation requires "linkedom". ' +
+        'Install it as a dependency or pass format: "text" to skip transformation.',


This might be better if there was a supported way to get raw html (untransformed), for people who don't want to import linkedom or who (for whatever reason) want the original data. How about a new format option, "rawhtml" or something like that?

(I'm writing this here because this error path is not something a builder will probably be excited to be in. The fix would mostly be elsewhere.)

... or add another parameter so that the format can stay "html". That feels like a better idea to me.

I like what you're processing. I've added a new commit to have the escape hatch allowing users to intentionally seek raw html versus transformed: aece62a

This PR is ready for re-review and re-consideration @davidfedor

A few things:

I'm curious, why linkedom as opposed to a more widely supported library like jsdom? Is there any risk of supply chain pollution with the newer library?

Have the docs been updated to reflect the need for a third party dependency?

Can the dependency be added as an optional peer dependency so it shows up in install logs?

I ask as someone who is doing RSC data loading, and will need to have this work server-side :)

Hey Bryson (@arinthros)!

Based on your comment, I've moved from linkedom to jsdom.

The YV dev docs will need to be updated immediatley after this (I will do that)

The dep has been added as an optional peer dep, so it can show up in install logs.

Anything else blocking this?

@arinthros following up ^

davidfedor · 2026-04-23T19:39:00Z

(FYI I've asked for thoughts from Bryson H; not sure if he's got cycles to contribute or not)

Add `transform` param to `getPassage` (default: true) so consumers can receive untransformed HTML without needing linkedom on the server. CSS now handles verse label spacing for raw HTML via ::after pseudo-element. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Amp-Thread-ID: https://ampcode.com/threads/T-019df363-8170-750b-866b-d30055111f9b Co-authored-by: Amp <amp@ampcode.com>

arinthros · 2026-05-06T19:11:36Z

@davidfedor @cameronapak this looks good to me, I just don't see an "approve" button in my UI. Approved by me!

cameronapak · 2026-06-03T17:42:21Z

Some added context on this ticket is: Why transformBibleHtml Exists — And Where It May Not Be Needed

If we can get this merged in, then I can write better docs for helping people use our HTML and styles without them having to manually transformBibleHtml on their end

greptile-apps · 2026-06-03T17:48:33Z

  "devDependencies": {
    "@internal/eslint-config": "workspace:*",
    "@internal/tsconfig": "workspace:*",
+    "@types/jsdom": "^28.0.1",


@types/jsdom@^28.0.1 is 4 major versions ahead of the pinned runtime "jsdom": "24.0.0" devDependency. TypeScript validates the code against jsdom 28 type definitions while the actual test/dev environment runs jsdom 24. If any jsdom 28 API used here was added, removed, or changed between versions 24 and 28, compile-time checking would pass while the code silently breaks at runtime. The type package should match the pinned runtime version.

Suggested change

"@types/jsdom": "^28.0.1",

"@types/jsdom": "^24.0.0",

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/core/package.json Line: 50 Comment: `@types/jsdom@^28.0.1` is 4 major versions ahead of the pinned runtime `"jsdom": "24.0.0"` devDependency. TypeScript validates the code against jsdom 28 type definitions while the actual test/dev environment runs jsdom 24. If any jsdom 28 API used here was added, removed, or changed between versions 24 and 28, compile-time checking would pass while the code silently breaks at runtime. The type package should match the pinned runtime version. ```suggestion "@types/jsdom": "^24.0.0", ``` How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

cameronapak changed the title ~~feat(core): auto-transform Bible HTML in getPassage~~ Auto-transform the Bible HTML from getPassage so the consumer doesn't have extra steps Apr 20, 2026

greptile-apps Bot reviewed Apr 20, 2026

View reviewed changes

Comment thread packages/core/src/bible-html-transformer.ts Outdated

Comment thread packages/core/src/bible.ts Outdated

cameronapak and others added 2 commits April 20, 2026 14:10

chore: add changeset for auto-transform Bible HTML

453b0d3

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

cameronapak commented Apr 20, 2026

View reviewed changes

davidfedor reviewed Apr 23, 2026

View reviewed changes

cameronapak and others added 2 commits April 24, 2026 11:23

chore: update changeset with transform opt-out and CSS fallback

20c1599

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

cameronapak requested a review from davidfedor April 24, 2026 16:51

cameronapak and others added 2 commits May 4, 2026 09:33

chore(core): replace linkedom with jsdom as optional peer dep

c1a61db

Amp-Thread-ID: https://ampcode.com/threads/T-019df363-8170-750b-866b-d30055111f9b Co-authored-by: Amp <amp@ampcode.com>

Merge branch 'main' into transform-bible-html

753776e

Merge branch 'main' into transform-bible-html

4494016

Merge branch 'main' into transform-bible-html

36e2adb

greptile-apps Bot reviewed Jun 3, 2026

View reviewed changes

Conversation

cameronapak commented Apr 20, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

changeset-bot Bot commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidfedor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidfedor commented Apr 23, 2026

Uh oh!

arinthros commented May 6, 2026

Uh oh!

cameronapak commented Jun 3, 2026

Uh oh!

greptile-apps Bot Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cameronapak commented Apr 20, 2026 •

edited by greptile-apps Bot

Loading

changeset-bot Bot commented Apr 20, 2026 •

edited

Loading