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

Export react-native-device-info to be compatible with destructured ES6 imports #727

Merged
merged 4 commits into from
Aug 29, 2019
Merged

Export react-native-device-info to be compatible with destructured ES6 imports #727

merged 4 commits into from
Aug 29, 2019

Conversation

Taym95
Copy link
Contributor

@Taym95 Taym95 commented Jul 22, 2019

Description

Fixed issue #723

Checklist

  • I have tested this on a device/simulator for each compatible OS
  • I added the documentation in README.md
  • I mentioned this change in CHANGELOG.md
  • I updated the typings files (deviceinfo.d.ts, deviceinfo.js.flow)
  • I updated the dummy web/test polyfill (default/index.js)
  • I added a sample use of the API (example/App.js)

@mikehardy
Copy link
Collaborator

I like the idea but I wish this didn't add essentially a duplicate entry for each function. Is there any way (breaking or non-breaking) to do this without duplicating each function entry? Just trying to keep the barrier as low as possible for people that propose PRs - we already have like 5 things we ask people to do :-)

@Taym95
Copy link
Contributor Author

Taym95 commented Jul 25, 2019

Yeah, I got your point, but unfortunately, this is the only way I found to implement destructured ES6 imports!

@mikehardy
Copy link
Collaborator

ok - I will likely merge but I want to think on it just a bit more by looking at other modules to see how they did it (usually I examine react-native-community/netinfo) - maybe there's nothing to be done but maybe there's an organizational trick I/we could use

@Taym95
Copy link
Contributor Author

Taym95 commented Jul 25, 2019

Yes absolutely, That is exactly what I did, I checked how react-native-community/netinfo implemented it, but maybe we can found something better.

@mikehardy
Copy link
Collaborator

Man, I guess it's just verbose then. Some things javascript is great at. Some things, not so much...module systems... 🤷‍♂️

@Taym95
Copy link
Contributor Author

Taym95 commented Jul 25, 2019

Yes exactly, I removed duplicate function implementation, so now contributor can add function to be exported :

export function getExample() {
  return RNDeviceInfo.example;
}

then add it to default export :

export default {
	//other functions
	...,
	getExample
}

@mikehardy mikehardy changed the base branch from master to v3 August 29, 2019 16:16
@mikehardy mikehardy merged commit 8e4fe78 into react-native-device-info:v3 Aug 29, 2019
@mikehardy
Copy link
Collaborator

Thanks for your patience! Big refactor and release coming up as a v3, this will be in it

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