Transform ESM rather than mock#31
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jest.config.ts`:
- Around line 125-130: The transform configuration currently uses ts-jest as a
plain string and the transformIgnorePatterns only whitelists
platform-bible-react and lucide-react; update the ts-jest configuration to
explicitly override allowJs (so ts-jest will compile .js/.mjs from node_modules
if those packages ship JS) by providing ts-jest options (either via the
transform entry as ['ts-jest', { tsconfig: { allowJs: true } }] or via the
globals/'ts-jest' config), keep the existing transformIgnorePatterns whitelist
for platform-bible-react and lucide-react, and be prepared to extend that
whitelist if transitive ESM deps fail to parse during the full test run.
In `@README.md`:
- Line 98: Update the README entry that currently reads "parsing PT0 interlinear
XML" to "parsing PT9 interlinear XML" so the documentation matches the actual
parser directory and schema (reference: src/parsers/pt9/ and pt9-xml.md); locate
the string in the README and replace "PT0" with "PT9" in that description.
🪄 Autofix (Beta)
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: dee307f3-b28d-4bf8-8931-fb64a21711ec
📒 Files selected for processing (3)
README.md__mocks__/platform-bible-react.tsxjest.config.ts
💤 Files with no reviewable changes (1)
- mocks/platform-bible-react.tsx
| transform: { | ||
| '\\.[jt]sx?$': 'ts-jest', | ||
| }, | ||
|
|
||
| /** Transform dependencies with ESM that Jest cannot parse. */ | ||
| transformIgnorePatterns: ['node_modules/(?!(platform-bible-react|lucide-react)/)'], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) Which package manager is in use?
fd -HI -t f -d 2 '^(package-lock\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lockb)$'
# 2) Does tsconfig enable allowJs (needed for ts-jest to transform .js in node_modules)?
fd -HI -t f '^tsconfig(\..*)?\.json$' --exec sh -c 'echo "=== $1 ==="; cat "$1"' _ {}
# 3) Look for ts-jest transform options in jest config(s) / package.json
rg -nP -C2 '\b(ts-jest|allowJs|useESM|isolatedModules)\b' --type=ts --type=json
# 4) Peek at the entry points / deps of the two packages being transformed
fd -HI -t f '^package\.json$' node_modules/platform-bible-react node_modules/lucide-react 2>/dev/null \
| xargs -I{} sh -c 'echo "=== {} ==="; jq "{name, type, main, module, exports, dependencies, peerDependencies}" {}'Repository: sillsdev/interlinearizer-extension
Length of output: 50391
🏁 Script executed:
# 1) Check the full jest.config.ts to see if there's a ts-jest allowJs override
cat jest.config.ts | head -150
# 2) Look for ts-jest preset or options
rg -A 10 -B 2 "ts-jest" jest.config.ts
# 3) Check if the two packages exist in node_modules
ls -la node_modules/platform-bible-react node_modules/lucide-react 2>&1 | head -20Repository: sillsdev/interlinearizer-extension
Length of output: 6137
🏁 Script executed:
# Check package.json for the two packages and jest/ts-jest versions
rg -A 20 '"dependencies"|"devDependencies"' package.json | head -40
# Check if there's any ts-jest preset or globals config in jest.config.ts
rg -i "preset|globals|jest" jest.config.ts
# Look at comment around the transform to see if there's rationale for the format
grep -B 5 -A 5 "Transform TS/TSX" jest.config.tsRepository: sillsdev/interlinearizer-extension
Length of output: 2544
🏁 Script executed:
# Check if platform-bible-react or lucide-react are referenced anywhere in the codebase
rg -i "platform-bible-react|lucide-react" src/ --type ts --type tsx --type json
# Check if there's a TODO or comment about adding these packages
rg -B 3 -A 3 "platform-bible-react|lucide-react" jest.config.ts
# Check git status or recent changes to understand context
git log --oneline -20 jest.config.ts 2>/dev/null | head -10Repository: sillsdev/interlinearizer-extension
Length of output: 347
🏁 Script executed:
# Check jest/ts-jest version to understand current capabilities
rg '"jest"|"ts-jest"' package.json --type json -A 1
# Search for any issues or comments in code about these packages or ESM
rg -i "esm|@module|import\.meta" src/ --type ts --type js 2>/dev/null | head -20Repository: sillsdev/interlinearizer-extension
Length of output: 260
Verify ts-jest config handles ESM from platform-bible-react and lucide-react.
The two packages are whitelisted to be transformed by ts-jest instead of mocked, but a few things need checking:
-
allowJsoverride: The maintsconfig.jsonhasallowJs: false, andjest.config.tsspecifies ts-jest as a plain string (no options object). This means ts-jest will inherit that setting and won't emit.jsfiles from node_modules. If either package ships.js/.mjsfiles, they'll fail to parse. Consider either:- Adding
allowJs: trueas an override in the ts-jest transform config, or - Verifying those packages export only TypeScript or pre-compiled ESM
- Adding
-
Transitive ESM dependencies: The negative-lookahead pattern only whitelists these two packages. If either re-exports from another ESM-only package (common for icon libraries or Radix/class-variance-authority), those will still be ignored and tests will fail at import time. Be prepared to extend the list once tests run and reveal missing deps.
Worth running the full test suite (not just a single file) to surface any of these issues before merging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jest.config.ts` around lines 125 - 130, The transform configuration currently
uses ts-jest as a plain string and the transformIgnorePatterns only whitelists
platform-bible-react and lucide-react; update the ts-jest configuration to
explicitly override allowJs (so ts-jest will compile .js/.mjs from node_modules
if those packages ship JS) by providing ts-jest options (either via the
transform entry as ['ts-jest', { tsconfig: { allowJs: true } }] or via the
globals/'ts-jest' config), keep the existing transformIgnorePatterns whitelist
for platform-bible-react and lucide-react, and be prepared to extend that
whitelist if transitive ESM deps fail to parse during the full test run.
| - `src/main.ts` is the main entry file for the extension (registers commands and wires interlinear XML) | ||
| - `src/types/interlinearizer.d.ts` is this extension's types file that defines how other extensions can use this extension through the `papi`. It is copied into the build folder | ||
| - `src/parsers/interlinearXmlParser.ts` parses interlinear XML into structured data (uses fast-xml-parser). The PT9 XML schema and parsed output are documented in `src/parsers/pt9-xml.md` | ||
| - `src/parsers/pt9/` contains parser and schema for parsing PT0 interlinear XML into structured data |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check parser directory contents and references to PT0/PT9 terminology
# Check what files exist in the pt9 directory
echo "=== Files in src/parsers/pt9/ directory ==="
fd . src/parsers/pt9/ --type f
# Search for PT0 references in parser code and docs
echo -e "\n=== References to 'PT0' in parser files ==="
rg -i 'pt0' src/parsers/ -C2
# Search for PT9 references in parser code and docs
echo -e "\n=== References to 'PT9' in parser files ==="
rg -i 'pt9' src/parsers/ -C2
# Check test data filenames for version indicators
echo -e "\n=== Test data files ==="
fd . test-data/ --type fRepository: sillsdev/interlinearizer-extension
Length of output: 2043
Fix terminology: "PT0" should be "PT9".
Line 98 states "parsing PT0 interlinear XML," but the directory src/parsers/pt9/ and all parser documentation (pt9-xml.md) consistently reference PT9. Update the description to "parsing PT9 interlinear XML" to match the actual schema and code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 98, Update the README entry that currently reads "parsing
PT0 interlinear XML" to "parsing PT9 interlinear XML" so the documentation
matches the actual parser directory and schema (reference: src/parsers/pt9/ and
pt9-xml.md); locate the string in the README and replace "PT0" with "PT9" in
that description.
|
I can't get this to work on my end. Here's what I found: The The cascade that follows:
Perhaps I'm misunderstanding something here. |
|
I'm not sure why the tests ran just fine for me locally with these changes, but it's probably not worth trying to sort out. |
2 participants
Suggestion for #30
An optional alternative to creating mocks for Jest-unfriendly deps.
This change is
Summary by CodeRabbit
Release Notes
Documentation
Tests