fix(core): surface jiti missing error and story import failures#152
fix(core): surface jiti missing error and story import failures#152zrosenbauer merged 4 commits intomainfrom
Conversation
When the `jiti` peer dependency is not installed, `createStoryImporter` now returns a Result error with a helpful install hint instead of crashing. The stories viewer also displays per-file import errors instead of a silent warning count. Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 73bfeb1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughcreateStoryImporter() now returns a readonly result tuple: readonly [Error, null] | readonly [null, StoryImporter]. It adds a resolveJiti() helper that uses Module.createRequire(import.meta.url) to dynamically require the optional jiti peer dependency and returns an install-instructions Error when jiti is missing. StoryImporter.importStory’s promise return type was made readonly for its tuple variants. Viewer call sites (StoriesCheck, StoriesOutput, StoriesViewer/StoriesScreen) destructure [importerError, importer] and short-circuit on importerError: StoriesCheck sets process.exitCode = 1, logs and exits; StoriesOutput sets an error phase and skips discovery; StoriesViewer enters an error discovery phase and renders import-failure details. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 `@packages/core/src/stories/importer.ts`:
- Around line 21-25: createStoryImporter currently changed its public signature
from returning a StoryImporter to a Result-style tuple, which is a breaking API
change; restore the original exported contract by reverting createStoryImporter
to return the original StoryImporter type (or throw/propagate errors the old
way) and move the new Result-returning behavior into a new symbol (e.g.,
createStoryImporterSafe or createStoryImporterResult) so external callers keep
the unchanged export; ensure the re-export of createStoryImporter (the public
symbol) remains the original signature and only the new helper is added/
exported for the breaking behavior.
- Around line 74-90: resolveJiti() uses bare require('jiti') which fails in ESM
(ReferenceError: require is not defined); replace the bare require with a
require created from Module.createRequire(import.meta.url) (Module is already
imported) so the code calls requireFromModule('jiti') instead of require; update
resolveJiti to use that created require and keep the same return tuple shape and
error message when loading still fails.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9eaa961f-a0b6-49aa-92e5-9aafd5cdae6d
⛔ Files ignored due to path filters (1)
.changeset/stories-jiti-check.mdis excluded by!.changeset/**
📒 Files selected for processing (4)
packages/core/src/stories/importer.tspackages/core/src/stories/viewer/stories-check.tsxpackages/core/src/stories/viewer/stories-output.tsxpackages/core/src/stories/viewer/stories-screen.tsx
Bare `require()` is not available in ESM contexts. Use `Module.createRequire(import.meta.url)` instead. Co-Authored-By: Claude <noreply@anthropic.com>
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 `@packages/core/src/stories/importer.ts`:
- Around line 25-27: The function createStoryImporter currently returns tuples
in the reversed `[Error, null] | [null, StoryImporter]` shape; change its Result
signature to the repo-standard `[StoryImporter | null, Error | null]` and update
any other tuples in the same file (the other return sites around the
createStoryImporter implementation) to match the `[data, error]` order so
callers receive [StoryImporter|null, Error|null] consistently; ensure the
exported type/signature and all return expressions (the branches that return
success or error) use StoryImporter|null first and Error|null second.
- Around line 77-93: In resolveJiti(), narrow the catch so it only synthesizes
the "install jiti" Error when the failure is a top-level MODULE_NOT_FOUND:
inspect the caught error from esmRequire('jiti') and if error.code ===
'MODULE_NOT_FOUND' and error.requireStack?.length === 0, return the custom
install-message Error and null as currently done; otherwise convert and return
the original error via toError(error) (so transitive missing deps,
ERR_REQUIRE_ESM, or module eval errors surface) while keeping the returned tuple
shape used for esmRequire/createJiti.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 964525e4-c859-4f8a-a7bb-4a496bb9eb49
📒 Files selected for processing (1)
packages/core/src/stories/importer.ts
Only show the "install jiti" hint when the top-level package is missing. Other errors (transitive deps, ERR_REQUIRE_ESM, eval failures) are passed through as-is via toError(). Co-Authored-By: Claude <noreply@anthropic.com>
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 `@packages/core/src/stories/importer.ts`:
- Line 27: The exported function createStoryImporter currently returns a mutable
tuple type; change its signature (and the matching types at the other two spots,
including StoryImporter.importStory) to use the repo-standard immutable Result
shape: use either the shared type alias Result<T, E = Error> or explicit
readonly tuples (readonly [Error, null] | readonly [null, StoryImporter]) so
callers cannot mutate the error/data slots; update the three occurrences
(createStoryImporter return type and the two other declarations referenced on
lines 46 and 76) to the readonly Result form to match the project's convention.
- Around line 27-32: createStoryImporter currently only returns a Result tuple
for resolveJiti failures but still performs synchronous initialization (using
createJiti and subsequent setup) that can throw instead of returning [Error,
null]; update createStoryImporter to wrap the rest of the setup in a try/catch
so any error thrown during initialization is caught and returned as [error,
null] rather than thrown, referencing createStoryImporter, resolveJiti,
jitiError, createJiti and StoryImporter to locate and protect the remaining
synchronous initialization before returning [null, StoryImporter].
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 95f8d12e-02d2-438d-a719-a5519a203fb5
📒 Files selected for processing (1)
packages/core/src/stories/importer.ts
- All Result tuple signatures now use `readonly` to match the project `Result<T, E>` convention. - The jiti setup path (installTsExtensionResolution + createJiti) is wrapped in try/catch so initialization errors surface as Result tuples instead of throws. Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Summary
createStoryImporter()now returns a Result tuple and surfaces a helpful error when thejitipeer dependency is missing (instead of silently failing)Test plan
pnpm typecheckpassespnpm test --filter=@kidd-cli/corepasses (1067 tests)kidd storieswithoutjitiinstalled → should show install hintkidd storieswithjitibut broken imports → should show per-file errors