-
-
Notifications
You must be signed in to change notification settings - Fork 56
[BridgeJS] Support default values #453
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
[BridgeJS] Support default values #453
Conversation
BridgeJS: Clean up BridgeJS: Simplified syntax for defaults in .js
d78c276
to
e144554
Compare
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.
First of all, thanks for working on this!
I have some concerns about the current approach before we merge. The main issue is that serializing default values to JavaScript requires explicit support for each literal type, which could become difficult to maintain. It's also inherently limited to values with 1:1 JS mappings, and we're passing the actual default value across the boundary when we really only need 1 bit of information (was the argument provided?), resulting in less perf efficiency.
The approach we used for Optional support might work better here: pass an isSome
flag for each parameter and evaluate defaults on the Swift side when isSome == false
. The downside is we'd need to generate multiple call-sites for each combination of default values (2^N combinations for N parameters):
switch (nameIsSome, ageIsSome) {
case (true, true): foo(String.lift(...), Int.lift(...))
case (true, false): foo(String.lift(...))
case (false, true): foo(age: Int.lift(...))
case (false, false): foo()
}
This could potentially be improved if we start using Swift macros, but I haven't found a clean solution yet.
Given that users can achieve similar functionality using Optional parameters, I'm wondering if the complexity-to-benefit ratio justifies native default parameter support right now, and just asking users to use Optional
parameters. What do you think?
Thanks for the thoughtful feedback! I appreciate the concerns about maintenance and performance. Before this implementation, I've considered alternative approaches, including generating multiple functions overloads or adding new flag similar to Developer Experience & Natural Swift SyntaxThe current implementation allows developers to write natural Swift code that works seamlessly across the bridge: @JS func greet(name: String = "Joe") -> String This matches Swift's native semantics and requires no special accommodation for BridgeJS. The alternative (using Migration Path for Existing CodebasesMost Swift codebases heavily favor default parameters over optionals and internal default value resolution in their API design. For example, in our Khasm project, in order to properly support default values which are common in our model definition layer (which is one of the parts we share between different products, as business model definitions is a good start point to share between different range of products) we currently use I think use case where potential BridgeJS users already have a suite of Swift apps and some JS app they want to migrate, having ability to start with existing codebase without much modification on Swift side is valuable feature that could speed up initial migration especially for larger codebases. Flexibility Over Forced OptimizationWhile the
For example, lots of models in our business layer require ~ 10 parameters, each supporting default value.
The current implementation lets developers choose: use defaults for convenience, or use optionals for maximum efficiency. Maybe extending documentation could help developer guide this decision and understand tradeoffs of each solution. Maintenance ConcernsThe current implementation is already well-structured with the Performance ImpactI understand the optimization concern about serializing defaults. However :
Given above, would you consider keeping default parameter support with the current JavaScript-side approach? The goal for this extension is mostly to allow BridgeJS to adopt for existing Swift projects, while not restricting developers to optimize when needed (by omitting default values and using optionals). |
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.
Thank you for the detailed explanation! If you have a lot of existing code with default parameter values, I think it makes sense to merge this so as not to block the adoption.
Introduction
This PR adds support for default parameter values when exporting Swift functions and constructors to JavaScript/TypeScript using the BridgeJS plugin.
Resolves: #452
Overview
param?: type
) in TypeScript, usingundefined
to trigger default value application, while not conflict with support for optionalsnil
, enum cases, and object initialization (e.g.MyClass("arg", 42)
)Examples
Generated JavaScript:
Generated TypeScript:
Testing
Added tests for different scenarios covering all supported default value types, including parameters, constructors, negative numbers, optionals, enums, and object initialization.
Documentation
Added documentation in
Exporting-Swift-Default-Parameters.md
.