Skip to content

Commit 645767c

Browse files
authored
[kyo-config] FlagPlatform: browser-safe process probe (#1832)
## Problem `Flag` crashes on Scala.js in browsers. `FlagPlatform.env`/`envNames` bind the `process` global to a `val` before their `typeof` guard runs: ```scala val proc = js.Dynamic.global.process if (js.typeOf(proc) == "undefined" || ...) ... ``` Scala.js compiles the first line to a bare `process` identifier read. In Node that yields the process object, but in a browser (no bundler shim) it throws `ReferenceError: process is not defined` before the guard on the next line ever executes. Any browser app that resolves a flag during startup dies with that error. This is the same defect #1823 fixed in `kyo-core`'s `SystemPlatformSpecific`. That PR carried both modules; the merge took the `kyo-core` half, so `kyo-config` still has the crashing pattern. This is the remaining half. ## Solution Keep the `typeof` guard inline on the global selection, which Scala.js compiles to a plain `typeof process` expression (safe on undeclared identifiers by JS semantics), and only bind the `val` after the guard passes — the same idiom `osName()`/`osArch()` and the merged `System.env` already use. Extracted as a `hasProcess` probe since both readers need it. ## Tests `FlagPlatformTest` gains the missing-global topology, reproduced by deleting `process` from the global object for the duration of the read: `process` then becomes an *undeclared* identifier, which is exactly the state that makes a bare read throw while `typeof process` still answers `"undefined"`. Restoring it in a `finally` keeps the test runner (which talks over `process.stdout`) alive even when an assertion fails. Both new cases fail with `ReferenceError: process is not defined` against the current code and pass with the fix. `envNames` had no coverage at all before, so its Node-path case comes along. `SystemPlatformSpecificJsWasmTest` is new and adds the same coverage for the guard #1823 already merged — `env`, `osName` and `osArch` all read `process` and none of them had a test for the browser topology. ## Notes No behavior change on Node: the same values are returned for present, absent and null variables. The two modules are deliberately separate concerns here: `kyo-config` carries the fix, `kyo-core` only gains test coverage for code already on `main`.
1 parent 92448b6 commit 645767c

3 files changed

Lines changed: 97 additions & 8 deletions

File tree

kyo-config/js-wasm/src/main/scala/kyo/FlagPlatform.scala

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,37 @@ private[kyo] object FlagPlatform {
1111
def properties: Iterable[String] =
1212
java.lang.System.getProperties.propertyNames().asScala.map(_.toString).toList
1313

14+
/** Must stay INLINE on the global selection: only then does Scala.js emit a plain `typeof process`, which
15+
* JS semantics make safe on an undeclared identifier. Binding the selection to a val first emits a bare
16+
* `process` read, which throws `ReferenceError` before any guard can run.
17+
*/
18+
private def hasProcess: Boolean =
19+
js.typeOf(js.Dynamic.global.process) != "undefined"
20+
1421
// A host with no `process` global (a browser, or a Wasm host with no Node shim) or a missing variable
1522
// falls back to the stdlib read: `java.lang.System.getenv` always returns null under Scala.js-Node, but
1623
// a Wasm host may resolve it through its own environment binding, so the fallback still gives the
1724
// caller its best answer instead of a hardcoded null.
1825
def env(name: String): String = {
19-
val proc = js.Dynamic.global.process
20-
if (js.typeOf(proc) == "undefined" || js.typeOf(proc.env) == "undefined") java.lang.System.getenv(name)
26+
if (!hasProcess) java.lang.System.getenv(name)
2127
else {
22-
val value = proc.env.selectDynamic(name)
23-
if (js.isUndefined(value) || value == null) java.lang.System.getenv(name)
24-
else value.asInstanceOf[String]
28+
val proc = js.Dynamic.global.process
29+
if (js.typeOf(proc.env) == "undefined") java.lang.System.getenv(name)
30+
else {
31+
val value = proc.env.selectDynamic(name)
32+
if (js.isUndefined(value) || value == null) java.lang.System.getenv(name)
33+
else value.asInstanceOf[String]
34+
}
2535
}
2636
}
2737

2838
def envNames: Iterable[String] = {
29-
val proc = js.Dynamic.global.process
30-
if (js.typeOf(proc) == "undefined" || js.typeOf(proc.env) == "undefined") Iterable.empty
31-
else js.Object.keys(proc.env.asInstanceOf[js.Object]).toSeq
39+
if (!hasProcess) Iterable.empty
40+
else {
41+
val proc = js.Dynamic.global.process
42+
if (js.typeOf(proc.env) == "undefined") Iterable.empty
43+
else js.Object.keys(proc.env.asInstanceOf[js.Object]).toSeq
44+
}
3245
}
3346

3447
}

kyo-config/js-wasm/src/test/scala/kyo/FlagPlatformTest.scala

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,19 @@ import scala.scalajs.js
1111
*/
1212
class FlagPlatformTest extends AnyFreeSpec {
1313

14+
/** Runs `f` with `process` deleted from the global object, so it is an UNDECLARED identifier — the state a
15+
* browser is in, and the one where a bare read throws while `typeof process` still answers "undefined".
16+
* Restored in a `finally` because the test runner talks over `process.stdout`.
17+
*/
18+
private def withoutProcessGlobal[A](f: => A): A = {
19+
// `globalThis`, not `js.Dynamic.global`, which Scala.js allows only left of a `.`-selection.
20+
val global = js.Dynamic.global.globalThis
21+
val saved = js.Dynamic.global.process
22+
js.special.delete(global, "process")
23+
try f
24+
finally global.updateDynamic("process")(saved)
25+
}
26+
1427
"env" - {
1528
"reads a variable set in Node process.env" in {
1629
js.Dynamic.global.process.env.updateDynamic("KYO_FLAGPLATFORM_PROBE")("enabled")
@@ -23,6 +36,21 @@ class FlagPlatformTest extends AnyFreeSpec {
2336
"returns null for a name that is not set in Node process.env" in {
2437
assert(FlagPlatform.env("KYO_FLAGPLATFORM_UNSET") eq null)
2538
}
39+
40+
"falls back to the stdlib read with no process global, instead of throwing ReferenceError" in {
41+
assert(withoutProcessGlobal(FlagPlatform.env("KYO_FLAGPLATFORM_PROBE")) eq null)
42+
}
43+
}
44+
45+
"envNames" - {
46+
"lists the names Node process.env carries" in {
47+
js.Dynamic.global.process.env.updateDynamic("KYO_FLAGPLATFORM_NAMES_PROBE")("1")
48+
assert(FlagPlatform.envNames.exists(_ == "KYO_FLAGPLATFORM_NAMES_PROBE"))
49+
}
50+
51+
"is empty with no process global, instead of throwing ReferenceError" in {
52+
assert(withoutProcessGlobal(FlagPlatform.envNames).isEmpty)
53+
}
2654
}
2755

2856
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package kyo.internal
2+
3+
import kyo.*
4+
import kyo.AllowUnsafe.embrace.danger
5+
import scala.scalajs.js as sjs
6+
7+
class SystemPlatformSpecificJsWasmTest extends kyo.test.Test[Any]:
8+
9+
/** Runs `f` with `process` deleted from the global object, so it is an UNDECLARED identifier — the state a
10+
* browser is in, and the one where a bare read throws while `typeof process` still answers "undefined".
11+
* Restored in a `finally` because the test runner talks over `process.stdout`.
12+
*/
13+
private def withoutProcessGlobal[A](f: => A): A =
14+
// `globalThis`, not `sjs.Dynamic.global`, which Scala.js allows only left of a `.`-selection.
15+
val global = sjs.Dynamic.global.globalThis
16+
val saved = sjs.Dynamic.global.process
17+
sjs.special.delete(global, "process")
18+
try f
19+
finally global.updateDynamic("process")(saved)
20+
end try
21+
end withoutProcessGlobal
22+
23+
"with no process global" - {
24+
"env returns null instead of throwing ReferenceError" in {
25+
assert(withoutProcessGlobal(SystemPlatformSpecific.env("PATH")) == null)
26+
}
27+
28+
"osName falls back to the empty string instead of throwing ReferenceError" in {
29+
assert(withoutProcessGlobal(SystemPlatformSpecific.osName()) == "")
30+
}
31+
32+
"osArch falls back to the empty string instead of throwing ReferenceError" in {
33+
assert(withoutProcessGlobal(SystemPlatformSpecific.osArch()) == "")
34+
}
35+
}
36+
37+
"on Node" - {
38+
"env resolves a variable set in process.env" in {
39+
sjs.Dynamic.global.process.env.updateDynamic("KYO_SYSTEMPLATFORM_PROBE")("enabled")
40+
assert(SystemPlatformSpecific.env("KYO_SYSTEMPLATFORM_PROBE") == "enabled")
41+
}
42+
43+
"env returns null for a name that is not set" in {
44+
assert(SystemPlatformSpecific.env("KYO_SYSTEMPLATFORM_UNSET") == null)
45+
}
46+
}
47+
48+
end SystemPlatformSpecificJsWasmTest

0 commit comments

Comments
 (0)