Skip falsy markers#204

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

istarkov merged 1 commit intogoogle-map-react:masterfromshark0der:master

Jul 24, 2016

Merged

Skip falsy markers#204

istarkov merged 1 commit intogoogle-map-react:masterfromshark0der:master

Jul 24, 2016

Conversation

Copy link

Contributor

shark0der commentedJul 24, 2016

Fixes#203

istarkov reviewed

Jul 24, 2016

View reviewed changes

src/google_map_markers.js

		@@ -229,6 +229,7 @@ export default class GoogleMapMarkers extends Component {
		this.dimesionsCache_ = {};

		const markers = React.Children.map(this.state.children, (child, childIndex) => {
		if (!child) return null;

Copy link

Collaborator

istarkovJul 24, 2016

Choose a reason for hiding this comment

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

why null? why not undefined or why not a filter like util

Copy link

ContributorAuthor

shark0derJul 24, 2016

Choose a reason for hiding this comment

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

I've commented on the issue as well:

I've returned null if child is falsy. The resulting array is passed to react so no need to filter out them after map.

Copy link

dapetcu21Jul 24, 2016•
edited
Loading

Choose a reason for hiding this comment

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

Also, I can see this library being used with big amounts of markers, so minimising the amount of steps of intermediate array construction (with.map and.filter), might be a good thing.

Copy link

Collaborator

istarkovJul 24, 2016

Choose a reason for hiding this comment

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

I don't likenulls ;-) the same result we will get withundefined.
So the question was about doesnull here has any advantage or hidden sense overundefined.

Having ES6 usingnull in any chain like env prevents me to use arguments defaults in the future, so I prefer not to usenull at all.

So please change null on undefined ;-)