TranscryptOrg / Transcrypt

Python 3.9 to JavaScript compiler - Lean, fast, open!
https://www.transcrypt.org
Apache License 2.0
2.86k stars 215 forks source link

Default parameters on async functions do not work in Safari #881

Open JGreenlee opened 2 months ago

JGreenlee commented 2 months ago

Consider these two functions.

def multiply(a, b=1):
    return a * b

async def multiply_async(a, b=1):
    return a * b

Compiled to JS, they are:

// Transcrypt'ed from Python, 2024-09-01 01:05:45
import {AssertionError, AttributeError, BaseException, DeprecationWarning, Exception, IndexError, IterableError, KeyError, NotImplementedError, RuntimeWarning, StopIteration, UserWarning, ValueError, Warning, __JsIterator__, __PyIterator__, __Terminal__, __add__, __and__, __call__, __class__, __envir__, __eq__, __floordiv__, __ge__, __get__, __getcm__, __getitem__, __getslice__, __getsm__, __gt__, __i__, __iadd__, __iand__, __idiv__, __ijsmod__, __ilshift__, __imatmul__, __imod__, __imul__, __in__, __init__, __ior__, __ipow__, __irshift__, __isub__, __ixor__, __jsUsePyNext__, __jsmod__, __k__, __kwargtrans__, __le__, __lshift__, __lt__, __matmul__, __mergefields__, __mergekwargtrans__, __mod__, __mul__, __ne__, __neg__, __nest__, __or__, __pow__, __pragma__, __pyUseJsNext__, __rshift__, __setitem__, __setproperty__, __setslice__, __sort__, __specialattrib__, __sub__, __super__, __t__, __terminal__, __truediv__, __withblock__, __xor__, _sort, abs, all, any, assert, bin, bool, bytearray, bytes, callable, chr, delattr, dict, dir, divmod, enumerate, filter, float, getattr, hasattr, hex, input, int, isinstance, issubclass, len, list, map, max, min, object, oct, ord, pow, print, property, py_TypeError, py_iter, py_metatype, py_next, py_reversed, py_typeof, range, repr, round, set, setattr, sorted, str, sum, tuple, zip} from './org.transcrypt.__runtime__.js';
var __name__ = '__main__';
export var multiply = function (a, b) {
    if (typeof b == 'undefined' || (b != null && b.hasOwnProperty ("__kwargtrans__"))) {;
        var b = 1;
    };
    return a * b;
};
export var multiply_async = async function (a, b) {
    if (typeof b == 'undefined' || (b != null && b.hasOwnProperty ("__kwargtrans__"))) {;
        var b = 1;
    };
    return a * b;
};

Now, let's test those 2 functions:

console.log('multiply')
console.log(multiply(5, 2))
console.log(multiply(5))

console.log('multiply_async:')
multiply_async(5, 2).then(console.log)
multiply_async(5).then(console.log)

In Chrome and Node, the output is:

multiply:
10
5
multiply_async:
10
5

But in Safari, the output is:

multiply:
10
5
multiply_async:
5
5

This is due to redeclaring b (var b = 1;), when b was already defined as the parameter of the function. In normal written Javascript, we would just reassign (b = 1).

Someone else has noticed this before: https://medium.com/@alsotang/a-hoisting-bug-in-the-async-function-in-safari-ba1ecc386b4a

Therefore, it is not safe to re-declare function parameters using var

JGreenlee commented 2 months ago

This patch will prevent function parameters from being re-declared with var

diff --git a/transcrypt/modules/org/transcrypt/compiler.py b/transcrypt/modules/org/transcrypt/compiler.py
index 05dff4f1..982ffd12 100644
--- a/transcrypt/modules/org/transcrypt/compiler.py
+++ b/transcrypt/modules/org/transcrypt/compiler.py
@@ -1020,7 +1020,7 @@ class Generator (ast.NodeVisitor):
                 self.emit ('if (typeof {0} == \'undefined\' || ({0} != null && {0}.hasOwnProperty ("__kwargtrans__"))) {{;\n', self.filterId (arg.arg))

                 self.indent ()
-                self.emit ('var {} = ', self.filterId (arg.arg))
+                self.emit ('{} = ', self.filterId (arg.arg))
                 self.visit (expr)
                 self.emit (';\n')
                 self.dedent ()
@@ -1161,6 +1161,8 @@ class Generator (ast.NodeVisitor):
                             self.emit ('{}.'.format ('.'.join ([scope.node.name for scope in self.getAdjacentClassScopes ()]))) # The target is a class attribute
                         elif target.id in self.getScope () .nonlocals:
                             pass
+                        elif (fnscope := self.getScope(ast.FunctionDef, ast.AsyncFunctionDef)) and target.id in [a.arg for a in fnscope.node.args.args]:
+                            pass
                         else:
                             if type (self.getScope () .node) == ast.Module: # Redundant but regular
                                 if hasattr (node, 'parentNode') and type (node.parentNode) == ast.Module and not target.id in self.allOwnNames:

So the output JS will be:

// Transcrypt'ed from Python, 2024-09-01 01:57:08
import {AssertionError, AttributeError, BaseException, DeprecationWarning, Exception, IndexError, IterableError, KeyError, NotImplementedError, RuntimeWarning, StopIteration, UserWarning, ValueError, Warning, __JsIterator__, __PyIterator__, __Terminal__, __add__, __and__, __call__, __class__, __envir__, __eq__, __floordiv__, __ge__, __get__, __getcm__, __getitem__, __getslice__, __getsm__, __gt__, __i__, __iadd__, __iand__, __idiv__, __ijsmod__, __ilshift__, __imatmul__, __imod__, __imul__, __in__, __init__, __ior__, __ipow__, __irshift__, __isub__, __ixor__, __jsUsePyNext__, __jsmod__, __k__, __kwargtrans__, __le__, __lshift__, __lt__, __matmul__, __mergefields__, __mergekwargtrans__, __mod__, __mul__, __ne__, __neg__, __nest__, __or__, __pow__, __pragma__, __pyUseJsNext__, __rshift__, __setitem__, __setproperty__, __setslice__, __sort__, __specialattrib__, __sub__, __super__, __t__, __terminal__, __truediv__, __withblock__, __xor__, _sort, abs, all, any, assert, bin, bool, bytearray, bytes, callable, chr, delattr, dict, dir, divmod, enumerate, filter, float, getattr, hasattr, hex, input, int, isinstance, issubclass, len, list, map, max, min, object, oct, ord, pow, print, property, py_TypeError, py_iter, py_metatype, py_next, py_reversed, py_typeof, range, repr, round, set, setattr, sorted, str, sum, tuple, zip} from './org.transcrypt.__runtime__.js';
var __name__ = '__main__';
export var multiply = function (a, b) {
    if (typeof b == 'undefined' || (b != null && b.hasOwnProperty ("__kwargtrans__"))) {;
        b = 1;
    };
    return a * b;
};
export var multiply_async = async function (a, b) {
    if (typeof b == 'undefined' || (b != null && b.hasOwnProperty ("__kwargtrans__"))) {;
        b = 1;
    };
    return a * b;
};

and Safari will now give the expected behavior


Besides default values for parameters, it also addresses explicit re-assignment of parameters, for example:

def multiply(a, b=1):
    if a < 0:
        a = 1
    return a * b

async def multiply_async(a, b=1):
    if a < 0:
        a = 1
    return a * b

will compile to:

// Transcrypt'ed from Python, 2024-09-01 02:00:23
import {AssertionError, AttributeError, BaseException, DeprecationWarning, Exception, IndexError, IterableError, KeyError, NotImplementedError, RuntimeWarning, StopIteration, UserWarning, ValueError, Warning, __JsIterator__, __PyIterator__, __Terminal__, __add__, __and__, __call__, __class__, __envir__, __eq__, __floordiv__, __ge__, __get__, __getcm__, __getitem__, __getslice__, __getsm__, __gt__, __i__, __iadd__, __iand__, __idiv__, __ijsmod__, __ilshift__, __imatmul__, __imod__, __imul__, __in__, __init__, __ior__, __ipow__, __irshift__, __isub__, __ixor__, __jsUsePyNext__, __jsmod__, __k__, __kwargtrans__, __le__, __lshift__, __lt__, __matmul__, __mergefields__, __mergekwargtrans__, __mod__, __mul__, __ne__, __neg__, __nest__, __or__, __pow__, __pragma__, __pyUseJsNext__, __rshift__, __setitem__, __setproperty__, __setslice__, __sort__, __specialattrib__, __sub__, __super__, __t__, __terminal__, __truediv__, __withblock__, __xor__, _sort, abs, all, any, assert, bin, bool, bytearray, bytes, callable, chr, delattr, dict, dir, divmod, enumerate, filter, float, getattr, hasattr, hex, input, int, isinstance, issubclass, len, list, map, max, min, object, oct, ord, pow, print, property, py_TypeError, py_iter, py_metatype, py_next, py_reversed, py_typeof, range, repr, round, set, setattr, sorted, str, sum, tuple, zip} from './org.transcrypt.__runtime__.js';
var __name__ = '__main__';
export var multiply = function (a, b) {
    if (typeof b == 'undefined' || (b != null && b.hasOwnProperty ("__kwargtrans__"))) {;
        b = 1;
    };
    if (a < 0) {
        a = 1;
    }
    return a * b;
};
export var multiply_async = async function (a, b) {
    if (typeof b == 'undefined' || (b != null && b.hasOwnProperty ("__kwargtrans__"))) {;
        b = 1;
    };
    if (a < 0) {
        a = 1;
    }
    return a * b;
};