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

Format map.fire call to match breaking change #771

Closed
wants to merge 1 commit into from
Closed

Conversation

tristen
Copy link
Member

@tristen tristen commented May 1, 2018

Taking the easy way out. A complete solution would re-write the draw tools API with its own draw.on('create') using something like EventEmitter.

ref #766

@mcwhittemore
Copy link
Contributor

@tristen - I'd like to have this to not be a breaking change. Is it possible for us to know which version of fire is in mapbox-gl?

@tristen
Copy link
Member Author

tristen commented May 1, 2018

@tristen - I'd like to have this to not be a breaking change. Is it possible for us to know which version of fire is in mapbox-gl?

This change still uses fire. It's just that its a private method now and argument signature has changed. The API should remain the same. I think this is close as we can get without dire breaking changes (although it does result in peerDependencies being pinned to [email protected])

@mcwhittemore
Copy link
Contributor

This PR currently requires all users of Draw to update to [email protected] which is the breaking change I'm noting.

@tristen
Copy link
Member Author

tristen commented May 1, 2018

OK. I guess we could look at mapboxgl.version and format the argument differently.

@mcwhittemore
Copy link
Contributor

we could do look at mapboxgl.version and format the argument differently.

Cool! This will probably set us up to do the EventEmitter option as well.

@mollymerp
Copy link
Contributor

thanks for taking this on @tristen – let me know if you need anything!

@tristen
Copy link
Member Author

tristen commented May 1, 2018

I think this still results in a breaking change. some version field would need to be passed as a required option

new MapboxDraw({
  mapboxglVersion: mapboxgl.version
});

Which we could helpfully throw as an error if the user hasn't passed one.

@tristen
Copy link
Member Author

tristen commented May 1, 2018

🚶 #772

@tristen tristen closed this May 1, 2018
@tristen tristen deleted the issue-766 branch May 1, 2018 21:11
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.

3 participants