Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
JGreenlee opened this issue Sep 1, 2024 · 1 comment · May be fixed by #882
Open

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

JGreenlee opened this issue Sep 1, 2024 · 1 comment · May be fixed by #882

Comments

@JGreenlee
Copy link

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
Copy link
Author

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;
};

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 a pull request may close this issue.

1 participant