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

Replace CommonJS exports with ES6 exports

Merged
merged 7 commits into from Jan 28, 2018

Conversation

hedgepigdaniel
Copy link
Contributor

@hedgepigdaniel hedgepigdaniel commented Jan 21, 2018

This allows for the use of react-native-maps without transpilation of modules to CommonJS. That means that it can be imported in projects that don't do such transpilation, which has the advantage of detection of undefined imports, dead code elimination, and generally better static analysis. See #1970 for an example.

Also in the future, it allows for static dead code elimination of code in react-native-maps since it will not be necessary to keep references to components that are not used, e.g. MapView.Marker, unless the user explicitly imported that component with

import { Marker } from 'react-native-maps';

All documentation and examples have been updated to use these imports but all the components are still attached to the base MapView component to avoid a breaking change.

I've checked all the examples - they work for me on Android. I can't see any reason why iOS would be different but I don't have an iOS device to check. I had to make one change to the Events example to prevent warnings relating to event pooling.

@alvelig
Copy link
Contributor

alvelig commented Jan 25, 2018

Ups, we have a little conflict here please :(

I will check it on the weekend on iOS (formally we need to do it) and merge then. Thank you very much.

@hedgepigdaniel
Copy link
Contributor Author

hedgepigdaniel commented Jan 26, 2018

Merged, and the AnimatedMarkers.js example which was changed in master still works.

@alvelig alvelig merged commit f553317 into react-native-maps:master Jan 28, 2018
1 check passed
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

2 participants