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.

Sign up for GitHub

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

Jump to bottom

[device] move modelName to native on iOS #14670

Merged
merged 10 commits into from
Oct 15, 2021
Merged

[device] move modelName to native on iOS #14670

merged 10 commits into from
Oct 15, 2021

Conversation

EvanBacon
Copy link
Contributor

@EvanBacon EvanBacon commented Oct 7, 2021 •
edited
Loading

Why

  • We are duplicating a lot of work, some of the values are even misaligned it seems.
  • In device and constants, we define the dict twice (4x in total), this PR reduces it to 2x.
  • Drop unsupported iPad and iPod models. We should probably also drop the Apple TV values.
  • Make it easier to copy/paste the expo-device changes into expo-constants.
  • Move the JS dictionary to native for iOS, this prevents a lot of useless JS from being bundled on Android and web, also simplifies the impl since we do the same on Android.
  • Fix deviceYear not being set properly on Simulators and using the latest year.

Test Plan

Checklist

  • Documentation is up to date to reflect these changes (eg: https://docs.expo.io and README.md).
  • This diff will work correctly for expo build (eg: updated @expo/xdl).
  • This diff will work correctly for expo prebuild & EAS Build (eg: updated a module plugin).

@EvanBacon EvanBacon requested review from bbarthec and ide as code owners October 7, 2021 20:24
@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Oct 7, 2021
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Oct 7, 2021
bbarthec
bbarthec approved these changes Oct 8, 2021
View reviewed changes
Copy link
Contributor

@bbarthec bbarthec left a comment

Choose a reason for hiding this comment

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

Great job with this one 🥳

@EvanBacon
Copy link
Contributor Author

EvanBacon commented Oct 8, 2021 •
edited
Loading

@brentvatne mentioned that we should just leave constants as-is so there's no breaking change for a deprecated feature.

@EvanBacon EvanBacon mentioned this pull request Oct 8, 2021
Merged
3 tasks
@brentvatne brentvatne merged commit df0dbc5 into master Oct 15, 2021
@brentvatne brentvatne deleted the @evanbacon/device/move-modelName-to-native branch October 15, 2021 17:46
brentvatne pushed a commit that referenced this pull request Oct 15, 2021
@EvanBacon @brentvatne
[device] move modelName to native on iOS (#14670)
f7dfd36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbarthec bbarthec bbarthec approved these changes

@ide ide ide approved these changes

@brentvatne brentvatne brentvatne approved these changes

Assignees
No one assigned
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@EvanBacon @brentvatne @ide @bbarthec @expo-bot