Skip to content

Commit 63cdb6b

Browse files
kateinoigakukunkateinoigakukun
authored
Merge pull request #346 from swiftwasm/yt/refactor-rawvalue-conversion
Stop using higher-order functions to convert JSValues to RawJSValues
2 parents d3b26d3 + 53676a9 commit 63cdb6b

File tree

8 files changed

+67
-81
lines changed

8 files changed

+67
-81
lines changed

‎Examples/Testing/package.json

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"devDependencies": {
3+
"playwright": "^1.52.0"
4+
}
5+
}

‎Makefile

-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ SWIFT_SDK_ID ?= wasm32-unknown-wasi
33
.PHONY: bootstrap
44
bootstrap:
55
npm ci
6-
npx playwright install
76

87
.PHONY: unittest
98
unittest:

‎Plugins/PackageToJS/Templates/package.json

+6-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@
1010
"dependencies": {
1111
"@bjorn3/browser_wasi_shim": "0.3.0"
1212
},
13-
"devDependencies": {
13+
"peerDependencies": {
1414
"playwright": "^1.51.0"
15+
},
16+
"peerDependenciesMeta": {
17+
"playwright": {
18+
"optional": true
19+
}
1520
}
1621
}

‎Plugins/PackageToJS/Tests/ExampleTests.swift

+31-24
Original file line numberDiff line numberDiff line change
@@ -114,20 +114,17 @@ extension Trait where Self == ConditionTrait {
114114
}
115115
}
116116

117+
typealias RunProcess = (_ executableURL: URL, _ args: [String], _ env: [String: String]) throws -> Void
117118
typealias RunSwift = (_ args: [String], _ env: [String: String]) throws -> Void
118119

119-
func withPackage(at path: String, body: (URL, _ runSwift: RunSwift) throws -> Void) throws {
120+
func withPackage(at path: String, body: (URL, _ runProcess: RunProcess, _ runSwift: RunSwift) throws -> Void) throws
121+
{
120122
try withTemporaryDirectory { tempDir, retain in
121123
let destination = tempDir.appending(path: Self.repoPath.lastPathComponent)
122124
try Self.copyRepository(to: destination)
123-
try body(destination.appending(path: path)) { args, env in
125+
func runProcess(_ executableURL: URL, _ args: [String], _ env: [String: String]) throws {
124126
let process = Process()
125-
process.executableURL = URL(
126-
fileURLWithPath: "swift",
127-
relativeTo: URL(
128-
fileURLWithPath: try #require(Self.getSwiftPath())
129-
)
130-
)
127+
process.executableURL = executableURL
131128
process.arguments = args
132129
process.currentDirectoryURL = destination.appending(path: path)
133130
process.environment = ProcessInfo.processInfo.environment.merging(env) { _, new in
@@ -157,13 +154,21 @@ extension Trait where Self == ConditionTrait {
157154
"""
158155
)
159156
}
157+
func runSwift(_ args: [String], _ env: [String: String]) throws {
158+
let swiftExecutable = URL(
159+
fileURLWithPath: "swift",
160+
relativeTo: URL(fileURLWithPath: try #require(Self.getSwiftPath()))
161+
)
162+
try runProcess(swiftExecutable, args, env)
163+
}
164+
try body(destination.appending(path: path), runProcess, runSwift)
160165
}
161166
}
162167

163168
@Test(.requireSwiftSDK)
164169
func basic() throws {
165170
let swiftSDKID = try #require(Self.getSwiftSDKID())
166-
try withPackage(at: "Examples/Basic") { packageDir, runSwift in
171+
try withPackage(at: "Examples/Basic") { packageDir, _, runSwift in
167172
try runSwift(["package", "--swift-sdk", swiftSDKID, "js"], [:])
168173
try runSwift(["package", "--swift-sdk", swiftSDKID, "js", "--debug-info-format", "dwarf"], [:])
169174
try runSwift(["package", "--swift-sdk", swiftSDKID, "js", "--debug-info-format", "name"], [:])
@@ -177,7 +182,10 @@ extension Trait where Self == ConditionTrait {
177182
@Test(.requireSwiftSDK)
178183
func testing() throws {
179184
let swiftSDKID = try #require(Self.getSwiftSDKID())
180-
try withPackage(at: "Examples/Testing") { packageDir, runSwift in
185+
try withPackage(at: "Examples/Testing") { packageDir, runProcess, runSwift in
186+
try runProcess(which("npm"), ["install"], [:])
187+
try runProcess(which("npx"), ["playwright", "install", "chromium-headless-shell"], [:])
188+
181189
try runSwift(["package", "--swift-sdk", swiftSDKID, "js", "test"], [:])
182190
try withTemporaryDirectory(body: { tempDir, _ in
183191
let scriptContent = """
@@ -208,27 +216,26 @@ extension Trait where Self == ConditionTrait {
208216
func testingWithCoverage() throws {
209217
let swiftSDKID = try #require(Self.getSwiftSDKID())
210218
let swiftPath = try #require(Self.getSwiftPath())
211-
try withPackage(at: "Examples/Testing") { packageDir, runSwift in
219+
try withPackage(at: "Examples/Testing") { packageDir, runProcess, runSwift in
212220
try runSwift(
213221
["package", "--swift-sdk", swiftSDKID, "js", "test", "--enable-code-coverage"],
214222
[
215223
"LLVM_PROFDATA_PATH": URL(fileURLWithPath: swiftPath).appending(path: "llvm-profdata").path
216224
]
217225
)
218226
do {
219-
let llvmCov = try which("llvm-cov")
220-
let process = Process()
221-
process.executableURL = llvmCov
222227
let profdata = packageDir.appending(
223228
path: ".build/plugins/PackageToJS/outputs/PackageTests/default.profdata"
224229
)
225-
let wasm = packageDir.appending(
226-
path: ".build/plugins/PackageToJS/outputs/PackageTests/TestingPackageTests.wasm"
230+
let possibleWasmPaths = ["CounterPackageTests.xctest.wasm", "CounterPackageTests.wasm"].map {
231+
packageDir.appending(path: ".build/plugins/PackageToJS/outputs/PackageTests/\($0)")
232+
}
233+
let wasmPath = try #require(
234+
possibleWasmPaths.first(where: { FileManager.default.fileExists(atPath: $0.path) }),
235+
"No wasm file found"
227236
)
228-
process.arguments = ["report", "-instr-profile", profdata.path, wasm.path]
229-
process.standardOutput = FileHandle.nullDevice
230-
try process.run()
231-
process.waitUntilExit()
237+
let llvmCov = try which("llvm-cov")
238+
try runProcess(llvmCov, ["report", "-instr-profile", profdata.path, wasmPath.path], [:])
232239
}
233240
}
234241
}
@@ -237,29 +244,29 @@ extension Trait where Self == ConditionTrait {
237244
@Test(.requireSwiftSDK(triple: "wasm32-unknown-wasip1-threads"))
238245
func multithreading() throws {
239246
let swiftSDKID = try #require(Self.getSwiftSDKID())
240-
try withPackage(at: "Examples/Multithreading") { packageDir, runSwift in
247+
try withPackage(at: "Examples/Multithreading") { packageDir, _, runSwift in
241248
try runSwift(["package", "--swift-sdk", swiftSDKID, "js"], [:])
242249
}
243250
}
244251

245252
@Test(.requireSwiftSDK(triple: "wasm32-unknown-wasip1-threads"))
246253
func offscreenCanvas() throws {
247254
let swiftSDKID = try #require(Self.getSwiftSDKID())
248-
try withPackage(at: "Examples/OffscrenCanvas") { packageDir, runSwift in
255+
try withPackage(at: "Examples/OffscrenCanvas") { packageDir, _, runSwift in
249256
try runSwift(["package", "--swift-sdk", swiftSDKID, "js"], [:])
250257
}
251258
}
252259

253260
@Test(.requireSwiftSDK(triple: "wasm32-unknown-wasip1-threads"))
254261
func actorOnWebWorker() throws {
255262
let swiftSDKID = try #require(Self.getSwiftSDKID())
256-
try withPackage(at: "Examples/ActorOnWebWorker") { packageDir, runSwift in
263+
try withPackage(at: "Examples/ActorOnWebWorker") { packageDir, _, runSwift in
257264
try runSwift(["package", "--swift-sdk", swiftSDKID, "js"], [:])
258265
}
259266
}
260267

261268
@Test(.requireEmbeddedSwift) func embedded() throws {
262-
try withPackage(at: "Examples/Embedded") { packageDir, runSwift in
269+
try withPackage(at: "Examples/Embedded") { packageDir, _, runSwift in
263270
try runSwift(
264271
["package", "--triple", "wasm32-unknown-none-wasm", "js", "-c", "release"],
265272
[

‎Sources/JavaScriptKit/ConvertibleToJSValue.swift

+16-35
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,10 @@ extension RawJSValue: ConvertibleToJSValue {
220220

221221
extension JSValue {
222222
func withRawJSValue<T>(_ body: (RawJSValue) -> T) -> T {
223+
body(convertToRawJSValue())
224+
}
225+
226+
fileprivate func convertToRawJSValue() -> RawJSValue {
223227
let kind: JavaScriptValueKind
224228
let payload1: JavaScriptPayload1
225229
var payload2: JavaScriptPayload2 = 0
@@ -232,7 +236,9 @@ extension JSValue {
232236
payload1 = 0
233237
payload2 = numberValue
234238
case .string(let string):
235-
return string.withRawJSValue(body)
239+
kind = .string
240+
payload1 = string.asInternalJSRef()
241+
payload2 = 0
236242
case .object(let ref):
237243
kind = .object
238244
payload1 = JavaScriptPayload1(ref.id)
@@ -252,53 +258,28 @@ extension JSValue {
252258
kind = .bigInt
253259
payload1 = JavaScriptPayload1(bigIntRef.id)
254260
}
255-
let rawValue = RawJSValue(kind: kind, payload1: payload1, payload2: payload2)
256-
return body(rawValue)
261+
return RawJSValue(kind: kind, payload1: payload1, payload2: payload2)
257262
}
258263
}
259264

260265
extension Array where Element: ConvertibleToJSValue {
261266
func withRawJSValues<T>(_ body: ([RawJSValue]) -> T) -> T {
262-
// fast path for empty array
263-
guard self.count != 0 else { return body([]) }
264-
265-
func _withRawJSValues(
266-
_ values: Self,
267-
_ index: Int,
268-
_ results: inout [RawJSValue],
269-
_ body: ([RawJSValue]) -> T
270-
) -> T {
271-
if index == values.count { return body(results) }
272-
return values[index].jsValue.withRawJSValue { (rawValue) -> T in
273-
results.append(rawValue)
274-
return _withRawJSValues(values, index + 1, &results, body)
275-
}
267+
let jsValues = map { $0.jsValue }
268+
// Ensure the jsValues live longer than the temporary raw JS values
269+
return withExtendedLifetime(jsValues) {
270+
body(jsValues.map { $0.convertToRawJSValue() })
276271
}
277-
var _results = [RawJSValue]()
278-
return _withRawJSValues(self, 0, &_results, body)
279272
}
280273
}
281274

282275
#if !hasFeature(Embedded)
283276
extension Array where Element == ConvertibleToJSValue {
284277
func withRawJSValues<T>(_ body: ([RawJSValue]) -> T) -> T {
285-
// fast path for empty array
286-
guard self.count != 0 else { return body([]) }
287-
288-
func _withRawJSValues(
289-
_ values: [ConvertibleToJSValue],
290-
_ index: Int,
291-
_ results: inout [RawJSValue],
292-
_ body: ([RawJSValue]) -> T
293-
) -> T {
294-
if index == values.count { return body(results) }
295-
return values[index].jsValue.withRawJSValue { (rawValue) -> T in
296-
results.append(rawValue)
297-
return _withRawJSValues(values, index + 1, &results, body)
298-
}
278+
let jsValues = map { $0.jsValue }
279+
// Ensure the jsValues live longer than the temporary raw JS values
280+
return withExtendedLifetime(jsValues) {
281+
body(jsValues.map { $0.convertToRawJSValue() })
299282
}
300-
var _results = [RawJSValue]()
301-
return _withRawJSValues(self, 0, &_results, body)
302283
}
303284
}
304285
#endif

‎Sources/JavaScriptKit/FundamentalObjects/JSString.swift

-9
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,4 @@ extension JSString {
9797
func asInternalJSRef() -> JavaScriptObjectRef {
9898
guts.jsRef
9999
}
100-
101-
func withRawJSValue<T>(_ body: (RawJSValue) -> T) -> T {
102-
let rawValue = RawJSValue(
103-
kind: .string,
104-
payload1: guts.jsRef,
105-
payload2: 0
106-
)
107-
return body(rawValue)
108-
}
109100
}

‎package-lock.json

+8-10
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
"@bjorn3/browser_wasi_shim": "^0.4.1",
3838
"@rollup/plugin-typescript": "^12.1.2",
3939
"@types/node": "^22.13.14",
40-
"playwright": "^1.51.0",
40+
"playwright": "^1.52.0",
4141
"prettier": "3.5.3",
4242
"rollup": "^4.37.0",
4343
"rollup-plugin-dts": "^6.2.1",

0 commit comments

Comments
 (0)