Skip to content

type specialize option for an edge case (fix #4930) #4960

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

Merged
merged 6 commits into from
Feb 19, 2021

Conversation

bobzhang
Copy link
Member

@bobzhang bobzhang commented Feb 19, 2021

@obj external f: (~lo: int, ~hi: string=?, unit) => _ = ""

let u = (lo, hi) => f(~lo, ~hi?, ())

Generated code was:

function u(lo, hi) {
  var tmp = {
    lo: lo
  };
  if (hi !== undefined) {
    tmp.hi = Caml_option.valFromOption(hi);
  }
  return tmp;
}

Now:

function u(lo, hi) {
  var tmp = {
    lo: lo
  };
  if (hi !== undefined) {
    tmp.hi = hi;
  }
  return tmp;
}

@bobzhang
Copy link
Member Author

bobzhang commented Feb 19, 2021

For the record, using spreading operators seem more elegant syntax wise:

return { lo,
              ... (optional1 === undefined ? {}, {optional1 },
              ... (optional2 === undefined ? {}, {optional2},
              ... (optional3 === undefined ? {}, {optional3}
            }

cc @rickyvetter IIRC, the optional parameter is used a lot in react bindings, this could have a large impact on those? https://github.com/rescript-lang/rescript-react/blob/master/src/ReactDOM.res#L57

@bobzhang bobzhang force-pushed the prototype_initialize_external_obj branch from e2d4395 to c757eff Compare February 19, 2021 10:03
@bobzhang bobzhang changed the title prototyping optimizing external obj type specialize option for an edge case (fix #4930) Feb 19, 2021
@bobzhang bobzhang merged commit b66fc0e into master Feb 19, 2021
@cristianoc cristianoc deleted the prototype_initialize_external_obj branch June 18, 2022 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant