React-google-maps: This lib is ruined by the author's fetish for recompose

[baebb]

In what universe is this a good way to write components?

compose(
  withProps({
    googleMapURL: "https://maps.googleapis.com/maps/api/js?key=AIzaSyC4R6AN7SmujjPUIGKdyao2Kqitzr1kiRg&v=3.exp&libraries=geometry,drawing,places",
    loadingElement: <div style={{ height: `100%` }} />,
    containerElement: <div style={{ height: `400px` }} />,
    mapElement: <div style={{ height: `100%` }} />,
  }),
  lifecycle({
    componentWillMount() {
      const refs = {}

      this.setState({
        bounds: null,
        center: {
          lat: 41.9, lng: -87.624
        },
        markers: [],
        onMapMounted: ref => {
          refs.map = ref;
        },
        onBoundsChanged: () => {
          this.setState({
            bounds: refs.map.getBounds(),
            center: refs.map.getCenter(),
          })
        },
        onSearchBoxMounted: ref => {
          refs.searchBox = ref;
        },
        onPlacesChanged: () => {
          const places = refs.searchBox.getPlaces();
          const bounds = new google.maps.LatLngBounds();

          places.forEach(place => {
            if (place.geometry.viewport) {
              bounds.union(place.geometry.viewport)
            } else {
              bounds.extend(place.geometry.location)
            }
          });
          const nextMarkers = places.map(place => ({
            position: place.geometry.location,
          }));
          const nextCenter = _.get(nextMarkers, '0.position', this.state.center);

          this.setState({
            center: nextCenter,
            markers: nextMarkers,
          });
          // refs.map.fitBounds(bounds);
        },
      })
    },
  }),
  withScriptjs,
  withGoogleMap
)(props =>
  <GoogleMap
    ref={props.onMapMounted}
    defaultZoom={15}
    center={props.center}
    onBoundsChanged={props.onBoundsChanged}
  >

[baebb]

Taken from the docs

[matheusnascgomes]

I agree, even though this libs is powerful i'm facing a lot of issues on understand that weird way.

[JustFly1984]

I Agree with you guys, that is a weirdest part of code in my project. Counting that React has all this new shiny nice API, It could be good to rewrite the code for performance reason too.

[phillipj]

@justrossthings could you please elaborate what you're hoping for when opening an issue like this?

[AonanLi]

You could choose to write a universal mapwrapper component and use that component instead of using recompose

[lkomar]

@justrossthings @JustFly1984 you can do the following in the separate file

export const decorate = compose(
  withProps({
    googleMapURL: "https://maps.googleapis.com/maps/api/js?key=AIzaSyC4R6AN7SmujjPUIGKdyao2Kqitzr1kiRg&v=3.exp&libraries=geometry,drawing,places",
    loadingElement: <div style={{ height: `100%` }} />,
    containerElement: <div style={{ height: `400px` }} />,
    mapElement: <div style={{ height: `100%` }} />,
  }),
  lifecycle({
    componentWillMount() {
      const refs = {}

      this.setState({
        bounds: null,
        center: {
          lat: 41.9, lng: -87.624
        },
        markers: [],
        onMapMounted: ref => {
          refs.map = ref;
        },
        onBoundsChanged: () => {
          this.setState({
            bounds: refs.map.getBounds(),
            center: refs.map.getCenter(),
          })
        },
        onSearchBoxMounted: ref => {
          refs.searchBox = ref;
        },
        onPlacesChanged: () => {
          const places = refs.searchBox.getPlaces();
          const bounds = new google.maps.LatLngBounds();

          places.forEach(place => {
            if (place.geometry.viewport) {
              bounds.union(place.geometry.viewport)
            } else {
              bounds.extend(place.geometry.location)
            }
          });
          const nextMarkers = places.map(place => ({
            position: place.geometry.location,
          }));
          const nextCenter = _.get(nextMarkers, '0.position', this.state.center);

          this.setState({
            center: nextCenter,
            markers: nextMarkers,
          });
          // refs.map.fitBounds(bounds);
        },
      })
    },
  }),
  withScriptjs,
  withGoogleMap
)

export default decorate;

remember to include imports for what you need in there, then just import decorate in the GoogleMap component:

import { decorate } from './mapDecorator';

...and use it like so:

decorate(props =>
  <GoogleMap
    ref={props.onMapMounted}
    defaultZoom={15}
    center={props.center}
    onBoundsChanged={props.onBoundsChanged}
  >

that's some simple HOC stuff

[lkomar]

Also, same goes for using multiple HOCs on exported component, or whatever you use. Wrapping it up in compose() is such a clean solution. As a quick example, compare just the docs of react-google-maps. You can wrap the component in 2 ways:

1. Without using compose():

import {
  withScriptjs,
  withGoogleMap,
  GoogleMap,
  Marker,
} from "react-google-maps";

const MapWithAMarker = withScriptjs(withGoogleMap(({
    googleMapURL: "https://maps.googleapis.com/maps/api/js?v=3.exp&libraries=geometry,drawing,places",
    loadingElement: <div style={{ height: `100%` }} />,
    containerElement: <div style={{ height: `400px` }} />,
    mapElement: <div style={{ height: `100%` }} />,
  }) =>

1. Using compose():

You have that in docs:

import { compose, withProps } from "recompose"
import { withScriptjs, withGoogleMap, GoogleMap, Marker } from "react-google-maps"

const MyMapComponent = compose(
  withProps({
    googleMapURL: "https://maps.googleapis.com/maps/api/js?v=3.exp&libraries=geometry,drawing,places",
    loadingElement: <div style={{ height: `100%` }} />,
    containerElement: <div style={{ height: `400px` }} />,
    mapElement: <div style={{ height: `100%` }} />,
  }),
  withScriptjs,
  withGoogleMap
)((props) =>

But I like the idea of storing the whole stuff in a variable, and wrapping the component in just the single variable:

import { compose, withProps } from "recompose"
import { withScriptjs, withGoogleMap, GoogleMap, Marker } from "react-google-maps"

const decorate = compose(
  withProps({
    googleMapURL: "https://maps.googleapis.com/maps/api/js?v=3.exp&libraries=geometry,drawing,places",
    loadingElement: <div style={{ height: `100%` }} />,
    containerElement: <div style={{ height: `400px` }} />,
    mapElement: <div style={{ height: `100%` }} />,
  }),
  withScriptjs,
  withGoogleMap
);

const MyMapComponent = decorate(props =>

Isn't that much cleaner?

[baebb]

@lkomar your example is out of touch with reality. Real gmaps implementations are already complex enough as it is and don't need another wrapper which forces you to refactor your code in an unfamiliar pattern.

If you like recompose- cool, that's great. You don't need to force everyone to learn a new paradigm to get their work done. For the sake of looking 'cleaner', might I add.

This is OSS! I don't need to make it easier for you to utlize the lib I made in my own time!

Software designed to make work easier which actually makes work harder due to a dependency is bad software. Don't be a bad software developer.

[JustFly1984]

compose is general kind of functions, you can get it from many npm packages including reduxramda` and others. Function composition is really useful programming abstraction, but it should be used only if it simplify readability, and does not introduce a barrier with adoption.

[JustFly1984]

To Everybody, who is interested: #949

[lkomar]

@JustFly1984 What I meant was improving the readability of the code, wrapping a function in a function in a function seems so hard for me to read, that when I've found out recompose lets me execute one after another in this type of scope when they're listed top to bottom I immediately liked it a lot. I agree it's a library, there are a lot of people who prefer utilizing as much built-in js functionalities as they can. The sole purpose of me writing the above was to explain an actual boost in readability, either it is by code splitting the huge chunk of code, either restrain from excessive HOC wrapping. Thanks you took time to share your point of view, I really appreciate it :beers:

As an offtop I can add that I didn't imagine recompose being a barrier at all, when I had to learn the whole react boilerplate stack after learning just react from tutorials. You know, components were fun, then there's state or lifecycle hooks blah blah.. And then I was thrown into a pit of redux, sagas, action creators and handlers, async communication with backend, HOC's and so on.

[lkomar]

@justrossthings Well to be honest.. I agree, despite appreciating the fact the author of lib presented how it can be done with recompose, he should've mainly focused on creating a library without making it a dependancy, more like an addition to it, if I understood you correctly. It's also a good lesson for me for my future development.

[JustFly1984]

If you'll look at current trends, HOF's are not in favor anymore, and there is other better approaches like "children as funcs" and "render props", "createContext" and "hooks"

[lkomar]

Would need to read more about that, but if that's what I think it is, I highly disagree with that. Idea of HOC/HOF's is reusability, it fits me just fine and makes the code so much cleaner. I never take trends into account, if, on the other hand, there is a good and solid argument against it, I'd happily consider changing it

[lkomar]

Moreover I'm good with this kind of syntax

const decorate = compose(
  withProps,
  connect(state => ({ someUserData: state.user.someUserField })),
  ...
)

export default decorate(SomeComponent);

than this f*ckery

const mapStateToProps = state => ({ someUserData: state.user.someUserField(state) });

export default withProps(connect(mapStateToProps)(someComponent));

especially that you can declare the variables (decorate in this case) before a function/class with that component, which further improves readability by another 100%, because you don't have to scroll to the very bottom, I keep it right after imports.

Both are fine, it's just a matter of which fits you better and why.

[JustFly1984]

Look at this example:
https://github.com/JustFly1984/react-google-maps-api/blob/master/examples/react-google-maps-api-gatsby/src/examples/directions-renderer-example.js

[JustFly1984]

@lkomar btw never pass lambda functions to connect() like you do in

const decorate = compose(
  withProps,
  connect(state => ({ someUserData: state.user.someUserField(state) })),
  ...
)

You always should declare

const mapStateToProps = state => ({
  ...
})

and
` const mapDispatchToProps = ({}) const mergeProps = ((mapStateProps, mapDispatchProps, ownProps, { ...options }) => ({}))
and pass that to connect()

that is called caching) have a nice day!

[lkomar]

I always do that, you didn't even say why not. The only proper way of writing functions with connect for me is

connect(state => ({ data: state.data }), { someActionCreator })

for caching I use reselect's createSelector()

the connect method looks then as follows

connect(state => ({ data: selectorForData(state) }), { stillSomeActionCreator })

why do you want to write mapStateToProps and mapDispatchToProps over and over again, if these are params of connect function, if you invoke a certain approach as a rule of thumb and you're consistent with it across your whole app, I can't see why I shouldn't pass these functions/parameters directly to connect, it shortens the amount of code I have to write, doesn't it? Afaik the effects is exactly the same.

[lkomar]

I see you're a type of guy who can't agree that somebody disagrees with your approach, despite having no solid arguments against it, like possible bugs. I find your approach time consuming and ineffective, should you provide some arguments why should I consider using one over another, I can rethink the case and reevaluate which is better.

[JustFly1984]

in case I provided you do not cache functions, and it is always better to cache functions too. Basically each time you create a function, you need to empty the memory after it is used and if it has no pointer, eventually Garbage collector will pick it up. If your component updates frequently - it is even worse. you are invoking GC more often than it could be, it takes more CPU cycles, and depends how big your app (it tends to grow over time), you get junky UX.

[baebb]

thanks for https://github.com/JustFly1984/react-google-maps-api @JustFly1984 . Going to be doing more work with gmaps & react in 2019 so I'll be trying your implementation out