Skip to content
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

Docs: add explanation for how to import components with default props. #573

Merged
merged 9 commits into from May 11, 2020
Merged

Docs: add explanation for how to import components with default props. #573

merged 9 commits into from May 11, 2020

Conversation

johnridesabike
Copy link
Contributor

This question comes up regularly on Discord.

Copy link

@woeps woeps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Just shadowing the makeProps function feels more lightweight to me. Having the original bindings around can be usefull if someone wanted to have several different derivations of the bound component.

@johnridesabike
Copy link
Contributor Author

johnridesabike commented May 7, 2020

Shadowing makeProps is an interesting technique. The compiled JS does look like it may be more lightweight too since it only adds a new function to the props, and doesn’t call React.createElement an extra time.

It feels a bit hacky to me though, so I was hesitant to put it in the PR. But I don’t mind adding it if we all think it’s worth mentioning.

module Greeting = {
   [@bs.module "./MyJavascriptFile.js"] [@react.component]
   external make: (~name: string) => React.element = "default";
   let makeProps = (~name="Peter", unit) => makeProps(~name, unit);
 };

let f = () => <Greeting  />

https://nit.sketch.sh/?code=LYewJgrgNgpgBAcQE4xgFwJYDsDmcC8cA3gFBzkDaAAgEYDOAdKJLHAEQMD0AsgJ4BSAQwBugugGMkGAA5oAYhlgMAVnTYBdONRSDxaBuJDBpILDCxp1ZcjAAeaGEiyCocYIIDWMAFxwAFAB+zsA+cHRoUrgAlAQAfHAASjC6+jCwIRYE7GAwAGaC0GhsANzWcLBobp4wAApIINJ0WYHBMPhsNeiObAA0cBBYGGgx+PHuXnUNdC2CIX0DQ1GlcAC+pSQVcLnNI-EAPMiomLjknLFAA

@peterpme peterpme added the docs issues that are about providing better documentation label May 7, 2020
@peterpme
Copy link
Collaborator

peterpme commented May 7, 2020

Hey thanks so much, just a few nits 😄

johnridesabike and others added 4 commits May 8, 2020 11:29
Co-authored-by: Peter Piekarczyk <peterpme@users.noreply.github.com>
Co-authored-by: Peter Piekarczyk <peterpme@users.noreply.github.com>
@peterpme
Copy link
Collaborator

Thank you!

@peterpme peterpme merged commit c4081d8 into reasonml:master May 11, 2020
@johnridesabike johnridesabike deleted the docs-opt-props branch May 11, 2020 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs issues that are about providing better documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants