Variable named await emitted in ESModule mode

[dispalt]

So this code translated to javascript https://github.com/typelevel/cats-effect/blob/series/3.x/kernel/shared/src/main/scala/cats/effect/kernel/GenConcurrent.scala#L74-L81 features something like

function $c_lcats_effect_kernel_genconcurrent$memoize$evaluating(await) {
  this.lcats_effect_kernel_genconcurrent$memoize$evaluating__f_await = null;
  this.lcats_effect_kernel_genconcurrent$memoize$evaluating__f_await = await
}

Shouldn't we escape that name? Since I think in strict mode its a reserved word?

[sjrd]

Does it actually fail to parse?

[sjrd]

It seems to be reserved in modules, indeed:
https://262.ecma-international.org/13.0/#sec-keywords-and-reserved-words

[dispalt]

By the browser, I think it works, but stricter minifiers like swc-project/swc#5359 apparently don't like it.

[sjrd]

There are two aspects to fixing this issue.

First, we should mangle any identifier that happens to be named await, like we do for many others at

scala-js/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/NameGen.scala

Lines 350 to 372 in 722d724

	/** Set of identifier names that cannot or should not be used for variable
	* names.
	*
	* This set includes and is limited to:
	*
	* - All ECMAScript 2015 keywords;
	* - Identifier names that are treated as keywords in ECMAScript 2015
	* Strict Mode;
	* - The identifiers `arguments` and `eval`, because they cannot be used for
	* local variable names in ECMAScript 2015 Strict Mode;
	* - The identifier `undefined`, because that's way too confusing if it does
	* not actually mean `void 0`, and who knows what JS engine performance
	* cliffs we can trigger with that.
	*/
	private final val ReservedJSIdentifierNames: Set[String] = Set(
	"arguments", "break", "case", "catch", "class", "const", "continue",
	"debugger", "default", "delete", "do", "else", "enum", "eval", "export",
	"extends", "false", "finally", "for", "function", "if", "implements",
	"import", "in", "instanceof", "interface", "let", "new", "null",
	"package", "private", "protected", "public", "return", "static", "super",
	"switch", "this", "throw", "true", "try", "typeof", "undefined", "var",
	"void", "while", "with", "yield"
	)

and similarly at

scala-js/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala

Lines 5948 to 5964 in 722d724

	private object FreshNameAllocator {
	/** List of local and label names that the emitter will avoid in JS
	* identifiers, and therefore will rewrite with non-ASCII characters.
	*
	* Since we're renaming all local and label symbols through fresh
	* allocators anyway, we take the opportunity to rename them in a nice way
	* (with ASCII characters only).
	*/
	private val EmitterReservedJSIdentifiers = List(
	"arguments", "break", "case", "catch", "class", "const", "continue",
	"debugger", "default", "delete", "do", "else", "enum", "eval",
	"export", "extends", "false", "finally", "for", "function", "if",
	"implements", "import", "in", "instanceof", "interface", "let", "new",
	"null", "package", "private", "protected", "public", "return",
	"static", "super", "switch", "this", "throw", "true", "try", "typeof",
	"undefined", "var", "void", "while", "with", "yield"
	)

That is fine, as it is only a codegen change that does not affect semantics.

The other thing is problematic: we must also forbid to use await as reference in the global scope. That means adding it to the list at

scala-js/ir/shared/src/main/scala/org/scalajs/ir/Trees.scala

Lines 854 to 875 in 722d724

	object JSGlobalRef {
	/** Set of identifier names that can never be accessed from the global
	* scope.
	*
	* This set includes and is limited to:
	*
	* - All ECMAScript 2015 keywords;
	* - Identifier names that are treated as keywords or reserved identifier
	* names in ECMAScript 2015 Strict Mode;
	* - The identifier `arguments`, because any attempt to refer to it always
	* refers to the magical `arguments` pseudo-array from the enclosing
	* function, rather than a global variable.
	*/
	final val ReservedJSIdentifierNames: Set[String] = Set(
	"arguments", "break", "case", "catch", "class", "const", "continue",
	"debugger", "default", "delete", "do", "else", "enum", "export",
	"extends", "false", "finally", "for", "function", "if", "implements",
	"import", "in", "instanceof", "interface", "let", "new", "null",
	"package", "private", "protected", "public", "return", "static",
	"super", "switch", "this", "throw", "true", "try", "typeof", "var",
	"void", "while", "with", "yield"
	)

which is tested at

scala-js/compiler/src/test/scala/org/scalajs/nscplugin/test/JSGlobalScopeTest.scala

Lines 329 to 338 in 722d724

	@Test
	def rejectAllReservedIdentifiers(): Unit = {
	val reservedIdentifiers = List(
	"arguments", "break", "case", "catch", "class", "const", "continue",
	"debugger", "default", "delete", "do", "else", "enum", "export",
	"extends", "false", "finally", "for", "function", "if", "implements",
	"import", "in", "instanceof", "interface", "let", "new", "null",
	"package", "private", "protected", "public", "return", "static",
	"super", "switch", "this", "throw", "true", "try", "typeof", "var",
	"void", "while", "with", "yield")

but doing so is an incompatible change! There could be codebases that already use await as a global ref. Either when emitting a NoModule or CommonJSModule, for which it is valid, or even when emitting an ESModule, since apparently browsers don't actually complain.

I am not sure what is the best course of action for the global refs. My best idea so far is to only warn about them at compile time (not link time), saying that they may cause downstream SyntaxErrors when emitting ES modules.