Repository navigation
fix(macro): let a user script that exports nothing finish its macro - #2209
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthrough
ChangesEmpty User Script Handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix · Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to User scripts that export nothing now finish and let the macro continue instead of raising an error. Selecting an empty member still errors. No merge-blocking risk was found. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
ESLint install failed: package-manager metadata or lockfile failed a supply-chain integrity policy. Refresh the packageManager pin and lockfile locally. 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. A rabbit ran a script at night Comment |
|
Independent verification: PASS Head: Surface: Obsidian app/installer 1.14.3 on macOS, isolated vault
|
Why
A user script that does all its work at the top level and exports nothing ran fine. QuickAdd then threw "is an empty object", showed an error notice, and stopped the macro, so later steps never ran. Fixes #2208.
What changed
Script::member) that is an empty object still raises the error.Scope
module.exports = { settings }), which still open the member picker. That behavior predates this PR.Tradeoffs
A script that defines a function but forgets to export it now does nothing silently, where before it showed this error. QuickAdd cannot tell that case apart from an intentional top-level script. The old error also fired only after the script had run, so it stopped the macro without undoing anything.
Blast Radius
Only user-script macro steps whose module exports nothing change behavior. The empty member picker from #404 still cannot open.
Verification
user script in macro for 'Todoist' is an empty object. A second test fails if the exception also covers drilled members.pnpm run testpassed 6851 tests.pnpm run lintandpnpm run buildpassed.Note
Fix: allow user scripts with no exports to finish their macro
A user script whose module loads as an empty record and has no selected member now returns with an undefined result instead of failing. The top-level code still runs first. Member-drilled selections that resolve to an empty object are still rejected by the existing validation.
Adds tests for both paths in userScriptExecution.test.ts, plus a
createApphelper that accepts a custom script source.Macroscope summarized 7fc1233.
Summary by CodeRabbit