Add Pretext virtualized text example - #1175
Conversation
Sorry, something went wrong.
📝 Walkthrough
WalkthroughThis PR adds a new Pretext text-measurement integration with complete documentation, a React virtualized chat example demonstrating the pattern, TypeScript configuration updates for Lit examples, and verification scripts for maintaining consistency across documentation and example builds. ChangesPretext Documentation, Example, and Verification Tooling
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2 ❌ Failed checks (2 warnings)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches 📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLintexamples/react/pretext/vite.config.js Parsing error: "parserOptions.project" has been provided for @typescript-eslint/parser. Comment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)examples/react/pretext/src/main.tsx (1)🤖 Prompt for all review comments with AI agents166-177: ⚡ Quick win
Conditionally access document.fonts.ready for older/quirky runtimes
In examples/react/pretext/src/main.tsx, the effect calls document.fonts.ready directly; since the Font Loading API (document.fonts / FontFaceSet.ready) is broadly “widely available” in modern browsers, this should not break typical targets. If you explicitly support older/embedded webviews where document.fonts may be missing, guard the access (e.g., only use .ready when document.fonts exists) to avoid a runtime throw.
🤖 Prompt for AI AgentsVerify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/react/pretext/src/main.tsx` around lines 166 - 177, The effect currently accesses document.fonts.ready unguarded which can throw in older/quirky runtimes; update the React.useEffect callback to first check for document.fonts (e.g., if (document.fonts && document.fonts.ready) { ... } else { ... }) and in the else branch run the same cleanup actions (preparedCache.clear(), clearCache(), setFontVersion(...)) or a safe fallback; keep the cancelled flag handling and ensure you attach the .then handler only when document.fonts.ready exists to avoid runtime errors.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/react/pretext/tsconfig.json`:
- Around line 2-4: Move the "composite" setting into the "compilerOptions"
object so it becomes an effective TypeScript compiler option: remove the
top-level "composite" key and add "composite": true inside the existing
"compilerOptions" block (ensure the key name and boolean value are preserved and
do not create a duplicate "compilerOptions" object).
In `@scripts/verify-links.ts`:
- Line 235: The script currently calls verifyLinks().catch(console.error) which
logs errors but leaves the process exit code as zero; change the top-level
invocation of verifyLinks to catch errors and exit non-zero on failure (e.g.,
verifyLinks().catch(err => { console.error(err); process.exit(1); })) so any
runtime exception causes the script/CI to fail; update the invocation where
verifyLinks() is called.
---
Nitpick comments:
In `@examples/react/pretext/src/main.tsx`:
- Around line 166-177: The effect currently accesses document.fonts.ready
unguarded which can throw in older/quirky runtimes; update the React.useEffect
callback to first check for document.fonts (e.g., if (document.fonts &&
document.fonts.ready) { ... } else { ... }) and in the else branch run the same
cleanup actions (preparedCache.clear(), clearCache(), setFontVersion(...)) or a
safe fallback; keep the cancelled flag handling and ensure you attach the .then
handler only when document.fonts.ready exists to avoid runtime errors.
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info ⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f6a1a7f7-fedd-43e1-bc6b-9f9d1b2a9a13
📥 CommitsReviewing files that changed from the base of the PR and between 693d915 and e66500c.
⛔ Files ignored due to path filters (1)- pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
- docs/config.json
- docs/pretext.md
- examples/lit/dynamic/tsconfig.json
- examples/lit/fixed/tsconfig.json
- examples/react/pretext/README.md
- examples/react/pretext/index.html
- examples/react/pretext/package.json
- examples/react/pretext/src/index.css
- examples/react/pretext/src/main.tsx
- examples/react/pretext/tsconfig.json
- examples/react/pretext/vite.config.js
- package.json
- scripts/verify-examples.ts
- scripts/verify-links.ts
Sorry, something went wrong.
| "composite": true, | ||
| "compilerOptions": { | ||
| "target": "ES2020", |
There was a problem hiding this comment.
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain🌐 Web query:
In TypeScript tsconfig.json, is "composite" a valid top-level property, or must it be inside "compilerOptions"?
💡 Result:
Answer: "composite" must be inside "compilerOptions" in a tsconfig.json; it is not a valid top-level tsconfig property. TypeScript’s official TSConfig reference lists a small set of top-level properties (such as "files", "extends", "include", "exclude", and "references") and then lists "composite" under the "compilerOptions" section [1]. Therefore, writing "composite" at the top level (e.g., { "composite": true }) is not supported by the tsconfig schema; it should be written like: { "compilerOptions": { "composite": true } } The official documentation for the "composite" option also describes it as a TSConfig option (i.e., a compiler option) and ties it to project references behavior rather than treating it as a top-level tsconfig field [2].
Citations:
- 1: https://www.typescriptlang.org/tsconfig/
- 2: https://www.typescriptlang.org/tsconfig/composite.html
Move "composite" under "compilerOptions" in tsconfig
In examples/react/pretext/tsconfig.json, "composite" is not a valid top-level tsconfig property; it must be placed under "compilerOptions" to take effect.
♻️ Proposed fix {
- "composite": true,
"compilerOptions": {
+ "composite": true,
"target": "ES2020",
"useDefineForClassFields": true,Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/react/pretext/tsconfig.json` around lines 2 - 4, Move the "composite" setting into the "compilerOptions" object so it becomes an effective TypeScript compiler option: remove the top-level "composite" key and add "composite": true inside the existing "compilerOptions" block (ensure the key name and boolean value are preserved and do not create a duplicate "compilerOptions" object).
Sorry, something went wrong.
| } | ||
| } | ||
|
|
||
| verifyLinks().catch(console.error) |
There was a problem hiding this comment.
⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Ensure unexpected errors fail the script with a non-zero exit code.
Line 235 currently logs errors but may still exit successfully on runtime exceptions, which can produce false-green CI.
Suggested fix-verifyLinks().catch(console.error)
+verifyLinks().catch((error) => {
+ console.error(error)
+ process.exit(1)
+})‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| verifyLinks().catch(console.error) | |
| verifyLinks().catch((error) => { | |
| console.error(error) | |
| process.exit(1) | |
| }) |
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/verify-links.ts` at line 235, The script currently calls
verifyLinks().catch(console.error) which logs errors but leaves the process exit
code as zero; change the top-level invocation of verifyLinks to catch errors and
exit non-zero on failure (e.g., verifyLinks().catch(err => { console.error(err);
process.exit(1); })) so any runtime exception causes the script/CI to fail;
update the invocation where verifyLinks() is called.
Sorry, something went wrong.
Reviewers
At least 0 approving reviews are required to merge this pull request.
Adds a React Pretext example, a robust usage guide, docs route validation, and all-example build verification. Verification: node scripts/verify-links.ts; node scripts/verify-examples.ts; Pretext example visual smoke via local Vite and headless Chrome.
Summary by CodeRabbit
New Features
Documentation
Chores