Spread Arg Support

[bbqbaron]

From #7, this adds support for parsing spread/variadic args in type annotations. Any Flow variadic becomes a BS named array. If it's optional, my tests are also expecting an extra unit arg, which as far as I can tell was already built into the code and therefore must be correct :).

Any Ocaml idioms I missed? Extra/dangerous test cases you can think of?

[rrdelaney]

Thanks so much for PR! I really appreciate you taking a shot at this! All the code looks good! The only problem is I don't think the bindings produced compile to the correct code 😕

Using this Flow code:

declare module 'spread-args' {
  declare export function foo(...bars: number[]): void
}

I get this binding:

external foo : bars::array float => unit = "" [@@bs.module "spread-args"];

But when I call it BuckleScript produces the following output:

var SpreadArgs = require("spread-args");

SpreadArgs.foo(/* float array */[
      1,
      2
    ]);

The call should look like:

var SpreadArgs = require("spread-args");

SpreadArgs.foo(
      1,
      2
    );

I think the right place to look in the manual is @@bs.splice

Thanks again for this awesome work 😄

[bbqbaron]

Oh, duh. Inexperienced with BS usage, sorry. Will check out splice, update tests and code shortly! On Sat, Jul 8, 2017 at 19:16 Ryan Delaney notifications@github.com wrote:

Thanks so much for PR! I really appreciate you taking a shot at this! All the code looks good! The only problem is I don't think the bindings produced compile to the correct code 😕

Using this Flow code:

declare module 'spread-args' { declare export function foo(...bars: number[]): void }

I get this binding:

external foo : bars::array float => unit = "" [@@bs.module "spread-args"];

But when I call it BuckleScript produces the following output:

var SpreadArgs = require("spread-args"); SpreadArgs.foo(/ float array /[ 1, 2 ]);

The call is should look like:

var SpreadArgs = require("spread-args"); SpreadArgs.foo( 1, 2 );

I think the right place to look in the manual is @@bs.splice https://bucklescript.github.io/bucklescript/Manual.html#_splice_calling_convention_bs_splice

Thanks again for this awesome work though 😄

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rrdelaney/ReasonablyTyped/pull/9#issuecomment-313886590, or mute the thread https://github.com/notifications/unsubscribe-auth/AGxb_ejt2hsrJb6hGCUawWTfxNn_RM9Oks5sMA3pgaJpZM4OQ6ad .

[rrdelaney]

Yea, I didn't know anything about BS either before starting this! I'm still learning stuff 🙂

The Reason playground is a really good place to test the output too

[bbqbaron]

Okay, so, back with many improvements.

• expanded BsType.Function to explicitly record its rest param, if any, since AFAICT BS does not have any way to record that any individual arg type is variadic.
• repositioned the printed unit for optional args to lie between the formal and rest params, since you can only bs.splice the last argument
• since said last argument can't be optional or defaulted, also suppressed optionality for rest params. I kept the Flow-side test cases that had optional variadics, to ensure that the resulting BS still behaved right.

[bbqbaron]

Ah; conflicts. Resolving soon.

[rrdelaney]

The conflicts should be from adding type params to functions - let me know if you need help 😄

[bbqbaron]

Sweet:

• Concat converted
• Renamed some vars to snake_case
• Merges resolved. Pretty sure I understood what was being done, which made it reasonably clear