BridgeJS: Allow extensions to contain types - #803
Conversation
|
I’ve pushed a commit to fix an existing .d.ts code-generation issue that the new tests triggered. |
| for skeleton in exportedSkeletons { | ||
| for structDef in skeleton.structs { | ||
| if structDef.name == name || structDef.swiftCallName == name { | ||
| return structDef.tsFullPath |
There was a problem hiding this comment.
Supporting bare names alongside qualified names makes sense for older skeletons. Could we prefer swiftCallName before falling back to name?
Right now declaration order can select the wrong struct. With Library.Shelf before a top-level Shelf, takeShelf(_:) and even the top-level initializer are emitted as using Library.Shelf.
A snapshot with nested and top-level structs sharing a name would cover this.
case .swiftStruct(let name):
let structs = exportedSkeletons.flatMap(\.structs)
let match =
structs.first { $0.swiftCallName == name }
?? structs.first { $0.name == name }
return match?.tsFullPath ?? type.tsType| } | ||
| return type.tsType | ||
| case .swiftHeapObject(let name): | ||
| for skeleton in exportedSkeletons { |
There was a problem hiding this comment.
The new resolver fixes direct references to nested classes and structs, but closure signatures still use BridgeType.tsType without the same resolution.
I tried a callback taking and returning Workshop.Bench. The declaration uses Workshop.Bench inside the callback even though only the top-level Bench interface exists.
Could closure parameters and returns go through resolveTypeScriptType as well? Extending NamespacedClassSignature with a callback would cover it.
case .closure(let signature, _):
let parameters = signature.parameters.enumerated().map { index, parameter in
let type = resolveTypeScriptType(parameter, exportedSkeletons: exportedSkeletons)
return "arg\(index): \(type)"
}.joined(separator: ", ")
let returnType = resolveTypeScriptType(
signature.returnType,
exportedSkeletons: exportedSkeletons
)
let resolvedReturnType =
signature.isAsync ? "Promise<\(returnType)>" : returnType
return "(\(parameters)) => \(resolvedReturnType)"| func resolveExtension(_ ext: ExtensionDeclSyntax) -> Bool { | ||
| let name = ext.extendedType.trimmedDescription | ||
| guard let extendedDecl = parent.typeDeclResolver.resolve(ext.extendedType) else { | ||
| return false |
There was a problem hiding this comment.
Deferred extension resolution works well for cross-file declarations, but a single pass leaves it dependent on source order.
I put extension Library.Shelf before the extension Library that declares Shelf. The generated struct was present, but the earlier extension's method was missing. Reversing the declarations restored it.
Could we retry unresolved extensions until a pass makes no progress? A reversed-order cross-file test would cover the same case across input files.
var pending = exportCollectors.flatMap { collector in
collector.deferredExtensions.map { (owner: collector, extension: $0) }
}
repeat {
let previousCount = pending.count
pending.removeAll { item in
exportCollectors.contains { $0.resolveExtension(item.extension) }
}
if pending.count == previousCount {
break
}
} while !pending.isEmpty
for item in pending {
item.owner.diagnoseUnresolvedExtension(item.extension)
}| } | ||
| if let components = qualifiedComponents(from: type) { | ||
| if let components = type.qualifiedComponents { | ||
| return lookupType(fullyQualified: components) |
There was a problem hiding this comment.
FYI, extension scoping now handles unqualified names, but relative qualified names such as Shelf.Divider are still treated as root-qualified. An @JS method using that spelling is omitted from the skeleton instead of resolving it as Library.Shelf.Divider.
This behavior already exists before this PR, so I opened #805 to follow it separately rather than treating it as part of this change.
Currently, you can write this:
but not this (which actually emitted broken generated code):
This PR removes this limitation, so you can define
@JStypes in extensions.