fix(repeat): declare repeat()'s closure signature as ANY, not VOID - #897
Merged
Conversation
DataType.VOID means "the associated JS value is undefined" (Python None), so Py2JS's type checker rejected any actual argument passed to a repeat()-composed closure - e.g. repeat(f, 5)(0) failed with "Expected argument 0 to have type 'NoneType', got 'int'". The closure is generic over its argument's type, so it should declare DataType.ANY, matching the existing pattern used for repl's identity- like closures. The bundle's own test suite didn't catch this because TestDataHandler.closure_call_unchecked calls the underlying JS function directly without validating declared argument types - only the real Py2JS evaluator enforces the signature. Fixes #896
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
repeat()built its composed closure withargs: [DataType.VOID], returnType: DataType.VOID.DataType.VOIDmeans "the associated JS value isundefined" (PythonNone), so Py2JS's type checker rejected any real argument passed to arepeat-composed closure:DataType.ANY, matching the existing pattern already used forrepl's identity-like closures (src/bundles/repl/src/index.ts:156,205).twice/thriceare thin wrappers overrepeatand are fixed the same way.Fixes #896
Why the existing tests didn't catch this
TestDataHandler.closure_call_unchecked(used by the bundle's JS-side test suite) calls the underlying JS function directly and never validates arguments against the closure's declared signature - only the real Py2JS evaluator enforces it. So this class of bug is invisible to the current unit tests; a signature-inspection helper in the shared test plugin would be needed to catch it there, which is out of scope for this fix.Test plan
yarn test(repeat bundle) — 5/5 existing tests passyarn tsc— no errorsyarn lint— clean