fix(yaml): export YAMLException type#73
Conversation
📝 WalkthroughWalkthroughAdds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 `@src/index.ts`:
- Around line 7-8: The current barrel re-exports `YAMLException` only as a type
which breaks runtime instanceof checks; change the exports so parseYAML and
stringifyYAML remain value exports, keep YAMLParseOptions and
YAMLStringifyOptions as type-only exports, and add a value export for the
YAMLException class (while optionally also exporting its type if desired) so
consumers can use `instanceof YAMLException`; update the export line referencing
YAMLException accordingly.
In `@src/yaml.ts`:
- Around line 51-52: The current line exports only the type alias, so the
runtime constructor for YAMLException is not exported and instanceof checks
fail; replace the type-only export with a value export of the class (i.e.,
export the YAMLException constructor itself rather than using "export type"), or
re-export the class from its defining module so the runtime value YAMLException
is available for instanceof checks; ensure the exported symbol name is
YAMLException to match callers.
|
Good catch @coderabbitai — applied both suggestions in caebf91. Changed \export type { YAMLException }\ → \export { YAMLException }\ in \yaml.ts, and split the barrel export in \index.ts\ so \YAMLException\ ships as a value export alongside the existing value exports. \instanceof YAMLException\ will now work correctly at runtime. |
|
Tip For best results, initiate chat on the files or code changes.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/yaml.ts`:
- Line 1: The project currently imports and re-exports YAMLException from
js-yaml (see the import line "import { load, dump, YAMLException } from
'js-yaml';" and the export of YAMLException later), but js-yaml is only in
devDependencies; update package.json to move js-yaml from devDependencies to
dependencies so consumers can import the re-exported YAMLException at runtime,
then run npm/yarn install to update lockfile.
| @@ -1,4 +1,4 @@ | |||
| import { load, dump } from "js-yaml"; | |||
| import { load, dump, YAMLException } from "js-yaml"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
PKG_FILE="$(fd '^package\.json$' | head -n 1)"
echo "Inspecting: ${PKG_FILE}"
jq -r '
{
dependencies_js_yaml: .dependencies["js-yaml"],
peerDependencies_js_yaml: .peerDependencies["js-yaml"],
devDependencies_js_yaml: .devDependencies["js-yaml"]
}
' "${PKG_FILE}"Repository: unjs/confbox
Length of output: 190
Move js-yaml from devDependencies to dependencies in package.json.
The code imports YAMLException as a value at Line 1 and re-exports it at Line 51. Since consumer code will use this export at runtime, js-yaml must be in dependencies, not devDependencies. Currently it is only in devDependencies.
Also applies to: 51-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/yaml.ts` at line 1, The project currently imports and re-exports
YAMLException from js-yaml (see the import line "import { load, dump,
YAMLException } from 'js-yaml';" and the export of YAMLException later), but
js-yaml is only in devDependencies; update package.json to move js-yaml from
devDependencies to dependencies so consumers can import the re-exported
YAMLException at runtime, then run npm/yarn install to update lockfile.
1 participant
Closes #17
Problem
YAMLExceptionis the error thrown byjs-yamlwhen YAML parsing fails, and it's already referenced in theYAMLParseOptions.onWarningcallback signature. However, it was declared locally via a privatedeclare class YAMLExceptionwithout anexportkeyword, making it impossible for users to import or catch the type explicitly:Fix
YAMLExceptionfromjs-yaml(already a dev dependency via@types/js-yaml)confbox/yamlconfboxentry point as welldeclare class YAMLExceptionand its supportinginterface MarkAfter this change, users can catch YAML parse errors with full type safety:
Summary by CodeRabbit