-
Notifications
You must be signed in to change notification settings - Fork 3
Auto-transform the Bible HTML from getPassage so the consumer doesn't have extra steps #216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
1c57836
789c2a1
453b0d3
aece62a
20c1599
c1a61db
753776e
4494016
36e2adb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| "@youversion/platform-core": minor | ||
| "@youversion/platform-react-hooks": minor | ||
| "@youversion/platform-react-ui": minor | ||
| --- | ||
|
|
||
| Auto-transform Bible HTML in `getPassage` — verse wrapping, footnote extraction, sanitization, and table fixes now happen automatically. Consumers no longer 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { z } from 'zod'; | ||
| import type { ApiClient } from './client'; | ||
| import { transformBibleHtml, type TransformBibleHtmlOptions } from './bible-html-transformer'; | ||
| import { BibleVersionSchema } from './schemas'; | ||
| import type { | ||
| BibleBook, | ||
|
|
@@ -13,6 +14,33 @@ import type { | |
| VOTD, | ||
| } from './types'; | ||
|
|
||
| async function getHtmlAdapters(): Promise<TransformBibleHtmlOptions> { | ||
| if (typeof globalThis.DOMParser !== 'undefined') { | ||
| return { | ||
| parseHtml: (h) => | ||
| new globalThis.DOMParser().parseFromString(h, 'text/html') as unknown as Document, | ||
| serializeHtml: (doc) => doc.body.innerHTML, | ||
| }; | ||
| } | ||
| let linkedom; | ||
| try { | ||
| linkedom = await import('linkedom'); | ||
| } catch { | ||
| throw new Error( | ||
| 'Server-side HTML transformation requires "linkedom". ' + | ||
| 'Install it as a dependency or pass format: "text" to skip transformation.', | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (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.)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ... or add another parameter so that the format can stay "html". That feels like a better idea to me.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A few things:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ask as someone who is doing RSC data loading, and will need to have this work server-side :)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey Bryson (@arinthros)! Based on your comment, I've moved from 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?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @arinthros following up ^ |
||
| ); | ||
| } | ||
| return { | ||
| parseHtml: (h) => | ||
| new linkedom.DOMParser().parseFromString( | ||
| `<html><body>${h}</body></html>`, | ||
| 'text/html', | ||
| ) as unknown as Document, | ||
| serializeHtml: (doc) => doc.body.innerHTML, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Client for interacting with Bible API endpoints. | ||
| */ | ||
|
|
@@ -234,18 +262,18 @@ export class BibleClient { | |
|
|
||
| /** | ||
| * Fetches a passage (range of verses) from the Bible using the passages endpoint. | ||
| * This is the new API format that returns HTML-formatted content. | ||
| * | ||
| * Note: The HTML returned from the API contains inline footnote content that should | ||
| * be transformed before rendering. Use `transformBibleHtml()` or | ||
| * `transformBibleHtmlForBrowser()` to clean up the HTML and extract footnotes. | ||
| * When format is "html" (the default), the returned content is automatically | ||
| * sanitized and transformed — verse content is wrapped for CSS targeting, | ||
| * footnotes are extracted into data attributes, and verse labels get | ||
| * non-breaking spaces. No manual call to `transformBibleHtml` is needed. | ||
| * | ||
| * @param versionId The version ID. | ||
| * @param usfm The USFM reference (e.g., "JHN.3.1-2", "GEN.1", "JHN.3.16"). | ||
| * @param format The format to return ("html" or "text", default: "html"). | ||
| * @param include_headings Whether to include headings in the content. | ||
| * @param include_notes Whether to include notes in the content. | ||
| * @returns The requested BiblePassage object with HTML content. | ||
| * @returns The requested BiblePassage object. | ||
| * | ||
| * @example | ||
| * ```ts | ||
|
|
@@ -258,9 +286,8 @@ export class BibleClient { | |
| * // Get an entire chapter | ||
| * const chapter = await bibleClient.getPassage(3034, "GEN.1"); | ||
| * | ||
| * // Transform HTML before rendering | ||
| * const passage = await bibleClient.getPassage(3034, "JHN.3.16", "html", true, true); | ||
| * const transformed = transformBibleHtmlForBrowser(passage.content); | ||
| * // Get plain text (no transformation applied) | ||
| * const text = await bibleClient.getPassage(3034, "JHN.3.16", "text"); | ||
| * ``` | ||
| */ | ||
| async getPassage( | ||
|
|
@@ -286,7 +313,18 @@ export class BibleClient { | |
| if (include_notes !== undefined) { | ||
| params.include_notes = include_notes; | ||
| } | ||
| return this.client.get<BiblePassage>(`/v1/bibles/${versionId}/passages/${usfm}`, params); | ||
| const passage = await this.client.get<BiblePassage>( | ||
| `/v1/bibles/${versionId}/passages/${usfm}`, | ||
| params, | ||
| ); | ||
|
|
||
| if (format === 'html') { | ||
| const adapters = await getHtmlAdapters(); | ||
| const { html } = transformBibleHtml(passage.content, adapters); | ||
| return { ...passage, content: html }; | ||
| } | ||
|
|
||
| return passage; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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>standaloneThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this
expect()call do regular expressions? we don't havepretags yet, that I know of, but still... it'd be great to tighten this up if that's not too difficult.