Skip to content

Allow GoogleApiWrapper() HOC to be initialized with data from props #172

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

Conversation

mruoss
Copy link
Contributor

@mruoss mruoss commented Mar 29, 2018

This pull requests enhances the GoogleApiWrapper HOC by the possibility to initialize it with data from props by accepting a function as argument when initializing the HOC. The function will be called by the HOC by passing it the props passed to the HOC.

This solution inspired by the recompose bundle (e.g. the HOC withProps()) and is completely backwards compatible.

With this change, the GoogleApiWrapper can be used the same way as before:

GoogleApiWrapper({
  apiKey: (YOUR_GOOGLE_API_KEY_GOES_HERE),
})(MapContainer)

Additionally it can be initialized with a function as described above:

GoogleApiWrapper((props) => ({
  apiKey: props.apiKey,
  language: props.language,
}))(MapContainer)

Or, as in our case, the HOC can be used with redux and recompose:

const mapStateToProps = (state) => ({
	lang: selectLang(state),
})

const enhance = compose(
	connect(mapStateToProps),
	GoogleApiWrapper(({ lang }) => ({
		apiKey: 'API_KEY',
		language: lang,
	})),
)

export default enhance(MapContainer)

@auser
Copy link
Contributor

auser commented Mar 29, 2018

I <3 this feature! Can you add a bit to the README describing this?

Michael Ruoss added 2 commits March 29, 2018 23:34
@mruoss
Copy link
Contributor Author

mruoss commented Mar 29, 2018

Absolutely. I added the simple example of the alternative configuration to the README. Also noticed I had based my branch on an old version of master. It should be free of conflicts now.

Edit: Oh well no wonder I was based on an old version - you pushed like 700 commits today ;)

class Wrapper extends React.Component {
constructor(props, context) {
super(props, context);
const options = typeof input === 'function' ? input(props) : input;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice idea, but I think that it's worth to rebuild it on props change as well. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. Wanna add a PR @rangoo94?

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem, I will do it today.

Copy link
Contributor

Choose a reason for hiding this comment

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

I added PR with update: #178

rangoo94 added a commit to rangoo94/google-maps-react that referenced this pull request Apr 6, 2018
rangoo94 added a commit to rangoo94/google-maps-react that referenced this pull request Apr 6, 2018
rangoo94 added a commit to rangoo94/google-maps-react that referenced this pull request Apr 6, 2018
rangoo94 added a commit to rangoo94/google-maps-react that referenced this pull request Apr 6, 2018
rangoo94 added a commit to rangoo94/google-maps-react that referenced this pull request Apr 6, 2018
rangoo94 added a commit to rangoo94/google-maps-react that referenced this pull request Apr 6, 2018
@auser auser merged commit d869f0a into fullstackreact:master Apr 7, 2018
auser added a commit that referenced this pull request Apr 7, 2018
#172 improvement: Allow Google HOC to be initialized from props
@mruoss
Copy link
Contributor Author

mruoss commented Apr 7, 2018

Thank you very much @rangoo94. My approach was indeed not complete.

@rangoo94
Copy link
Contributor

rangoo94 commented Apr 7, 2018

No problem, you're welcome :)

@mruoss mruoss deleted the feature/accept-functions-as-input branch April 9, 2018 06:27
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.

4 participants