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

Add docs for useConnect and update connect #61

Merged
merged 4 commits into from May 21, 2020
Merged

Add docs for useConnect and update connect #61

merged 4 commits into from May 21, 2020

Conversation

DAreRodz
Copy link
Member

Documentation for frontity/frontity#404

@DAreRodz
Copy link
Member Author

Some code was autoformatted, I guess that's not a problem for Gitbook, right?

cc: @juanmaguitar, @mburridge

```

It's a function that receives a React component an returns the same component but connected to the Frontity state, actions and libraries. Any instance of that component will receive three new props: `state`, `actions` and `libraries`, allowing the component to read the state, manipulate it through actions or use any code other packages have exposed in libraries. Also, that instance will re-render automatically whenever the value of `state` which the component is using is changed.

If you don't want to inject new props in your components, you can use the `injectProps` option set to `false`. That will make your component reactive to changes in the state but without passing new props. For these components to access the state use the [`useConnect`](frontity.md#useConnect) hook.
Copy link
Member

Choose a reason for hiding this comment

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

Could you clarify what you mean by "without passing new props" ? Which new props is it referring to? Does that mean that the value of the state props is never going to be updated in components with {injectProps: false} ?

Edit: I see that you clarify it below, but I would put it here as well 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll do that, thanks Michal 👍

@DAreRodz DAreRodz self-assigned this May 20, 2020
@juanmaguitar juanmaguitar self-requested a review May 20, 2020 15:18
@DAreRodz
Copy link
Member Author

I slightly changed the docs, I hope it's more clear now. @michalczaplinski, @juanmaguitar could you review this?

Copy link
Member

@michalczaplinski michalczaplinski left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@juanmaguitar juanmaguitar left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@DAreRodz
Copy link
Member Author

Thanks!

@DAreRodz DAreRodz merged commit 99c40d6 into code May 21, 2020
@DAreRodz DAreRodz deleted the use-connect branch May 21, 2020 17:46
@luisherranz
Copy link
Member

Hey guys, just to clarify this, people don't need to use { injectProps: false } to use useConnect. Only they want to avoid the props. So it's optional.

And I think it should not be recommended, except for the case where you want to pass down props to a HTML tag, like:

const Input = ({ state, ...restProps}) => {
   // Do something with `state`.

  return <input {...restProps} />
};

export default connect(Input);

Because in this case, input don't want that actions and libraries end up in the DOM.

const Input = (props) => {
    const { state } = useConnect();
   // Do something with `state`.

  return <input {...props} />
};

// In this case, avoid injecting `state`, `actions` and `libraries` so they are not in `...props`.
export default connect(Input, { injectProps: false });

The rest of the time, people should use just connect. This is perfectly fine:

const Input = ({ name, type }) => {
    const { state } = useConnect();
   // Do something with `state`.

  return <input name={name} type={type} />
};

export default connect(Input);

@juanmaguitar could you please clarifying this in the docs?

@juanmaguitar
Copy link
Member

juanmaguitar commented May 22, 2020

@luisherranz @DAreRodz clarification added → #71

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.

None yet

4 participants