Skip to content

fix: handle function eval input wrapping#56

Merged
kunchenguid merged 4 commits into
mainfrom
ezoss/fix-41-20260507-165346-diclnrap08zs-2
May 7, 2026
Merged

fix: handle function eval input wrapping#56
kunchenguid merged 4 commits into
mainfrom
ezoss/fix-41-20260507-165346-diclnrap08zs-2

Conversation

@kunchenguid
Copy link
Copy Markdown
Owner

Summary

  • Update eval/page helper wrapping to pass function and arrow inputs through while preserving supported no-arg IIFE compatibility.
  • Fix IIFE unwrapping edge cases for parenthesized calls and trailing semicolons so non-function calls are not rewritten incorrectly.
  • Align README, CLI help, and suggestions with the preferred direct function/arrow eval guidance.

Risk Assessment

✅ Low: The change is narrowly scoped to JavaScript expression wrapping for eval/run helpers, includes targeted coverage for prior edge cases, and does not introduce broad control-flow or integration changes.

Testing

  • Summary: Exercised the new JavaScript wrapping/IIFE behavior directly and then the full Vitest suite; all tests passed.
  • npm test -- test/run.test.ts test/wrap-js.test.ts
  • npm test
  • Outcome: ✅ passed across 1 run (30.5s)

Pipeline

Updates from git push no-mistakes

✅ **Rebase** - passed

Round 1 - passed ✅

🔧 **Review** - 1 issue found → auto-fixed (2)

Round 1 - found 1 warning

  • ⚠️ src/run.ts:64 - unwrapNoArgIIFE unwraps every parenthesized no-arg call before confirming the inner expression is actually a function literal. Valid eval inputs such as (window.getValue)() or (obj.method)() are now converted into () => (window.getValue)/() => (obj.method), so the function is returned instead of invoked. Only unwrap when the inner expression matches the supported function forms; otherwise preserve the original expression wrapping.

Round 2 (auto-fix) - found 1 warning

  • ⚠️ src/run.ts:63 - No-arg IIFEs with a trailing semicolon, such as (() => 1)();, are not unwrapped because rest must be exactly (). After that, the existing function-detection regex still treats the original string as a function and sends the IIFE text to evaluate_script, so a common copy-pasted IIFE form remains broken despite the new documented support. Accept (); here, or strip trailing semicolons before checking the no-arg call.

Round 3 (auto-fix) - passed ✅

✅ **Test** - passed

Round 1 - passed ✅

  • npm test -- test/run.test.ts test/wrap-js.test.ts
  • npm test
🔧 **Document** - 2 issues found → auto-fixed

Round 1 - found 2 issues (1 warning, 1 info)

  • ⚠️ src/suggestions.ts:75 - The user-facing eval tip still teaches multi-statement code by recommending an IIFE as the primary pattern. The change updates eval handling to pass arrow/function inputs through and unwrap no-arg IIFEs, and README/command help now document direct arrow/function input as the preferred multi-statement form with IIFE as accepted compatibility. Update this suggestion and its comment to match the new guidance.
  • ℹ️ src/cli.ts:241 - The run command's public script API help lists await page.eval(jsOrFn) but does not document that string inputs now use the same wrapping rules as the CLI eval command, including function passthrough and no-arg IIFE unwrapping. Since this change explicitly modifies page.eval behavior in createPageHelper, the run help should mention the accepted string/function forms or point to the eval syntax rules.

Round 2 (auto-fix) - passed ✅

✅ **Lint** - passed

Round 1 - passed ✅

✅ **Push** - passed

Round 1 - passed ✅

@kunchenguid kunchenguid force-pushed the ezoss/fix-41-20260507-165346-diclnrap08zs-2 branch from 105b901 to c0fd184 Compare May 7, 2026 18:30
@kunchenguid kunchenguid merged commit d9e6d9f into main May 7, 2026
3 checks passed
@kunchenguid kunchenguid deleted the ezoss/fix-41-20260507-165346-diclnrap08zs-2 branch May 7, 2026 18:46
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant