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

feat: upgrade to Expo 47 #299

Closed
wants to merge 2 commits into from
Closed

feat: upgrade to Expo 47 #299

wants to merge 2 commits into from

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented Nov 16, 2022

Checklist

Why

This package currently pulls in "@expo/config-plugins": "~5.0.0" and "@expo/config-types": "^46.0.1", which are both for SDK 46.

How

Looking at https://blog.expo.dev/expo-sdk-47-a0f6f5c038af, imports should be moved to expo package which would allow us to not have this PR for future SDK releases 🙂

Test Plan

Just TS - I haven't tried to apply this to a project.

import { ExpoConfig, PublishHook } from '@expo/config-types';
import { ConfigPlugin, createRunOncePlugin, WarningAggregator } from 'expo/config-plugins';
import type { ExpoConfig } from 'expo/config';
import type { PublishHook } from '@expo/config-types';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PublishHook doesn't exist in the re-exports from what I can see. It's not in the exported d.ts files though, so doesn't hurt (I moved @expo/config-types to devDependencies)

@@ -48,26 +46,27 @@
"rimraf": "^3.0.2"
},
"peerDependencies": {
"expo": "*",
"expo": ">=47.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is why I think this is a breaking change. In theory, future SDK updates won't, tho

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we probably don't want to use the re-exported ones for that reason, I don't think we should make it a breaking release just for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if #301 is breaking, then they can both go in?

Copy link
Contributor

Choose a reason for hiding this comment

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

If either of them is breaking, then both of them will have to wait for a bit. I'm not ready to create a fork in the road based on supported SDK versions, and ending up applying changes to both.

@kbrandwijk
Copy link
Contributor

Thank you very much for your contribution. I've wrapped these changes into #302, as those are also breaking changes and I'd rather ship a single major release.

@kbrandwijk kbrandwijk closed this Nov 23, 2022
@SimenB SimenB deleted the sdk-47 branch November 23, 2022 09:57
@SimenB SimenB restored the sdk-47 branch November 23, 2022 10:16
@SimenB SimenB mentioned this pull request Nov 23, 2022
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.

2 participants