-
-
Notifications
You must be signed in to change notification settings - Fork 63
BridgeJS: Remove BasicFormat usage in ExportSwift #617
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
Conversation
| return statements | ||
| statements.append("_swift_js_push_i32(Int32(\(accessor).count))") | ||
| let parsed: CodeBlockItemListSyntax = "\(raw: statements.joined(separator: "\n"))" | ||
| return Array(parsed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant copy here? Could this return some Sequence<CodeBlockItemSyntax> instead of you want it to avoid leaking CodeBlockItemListSyntax as a return type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just because the rest of StackCodegen expects [CodeBlockItemSyntax] now. This is unfortunate but it will be removed once we get rid of SwiftSyntax-based node construction at all.
| return statements | ||
| statements.append("_swift_js_push_i32(__bjs_isSome_\(varPrefix) ? 1 : 0)") | ||
| let parsed: CodeBlockItemListSyntax = "\(raw: statements.joined(separator: "\n"))" | ||
| return Array(parsed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
36aedb1 to
9162fa0
Compare
MaxDesiatov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunate that the diff is huge and there's unnecessary Array copying, but if it fixes the original issue, we can accept this as a temporary measure before subsequent optimizations are applied
Mitigate #512