chore(expo-passkeys): Remove expo-modules-core from dependencies in favor of Expo's bundled version
#5402
Conversation
🦋 Changeset detectedLatest commit: c441704 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
wobsoriano
changed the title
expo-modules-core to support Expo SDK 52expo-modules-core to support Expo SDK 52
wobsoriano
changed the title
expo-modules-core to support Expo SDK 52expo-modules-core from dependencies in favor of Expo's bundled version
|
!snapshot |
|
Hey @wobsoriano - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact |
| @@ -1,4 +1,4 @@ | |||
| import { requireNativeModule } from 'expo-modules-core'; | |||
| import { requireNativeModule } from 'expo'; | |||
There was a problem hiding this comment.
That would make the package compatible only with SDK 52, and we don't want that.
There was a problem hiding this comment.
I'm working under the assumption that this change actually makes it work with work with any Expo SDK version since our expo peerDeps is agnostic (*) and we only reference SDK 52 in devDeps.
Also, this approach mimics the structure of a freshly scaffolded Expo module via npx create-expo-module
What do you think?
There was a problem hiding this comment.
I think import { requireNativeModule } from 'expo'; is only available in SDK 52, at lease that's my understanding from their docs.
52
51
There was a problem hiding this comment.
Oh, interesting. Thanks!
I checked Expo SDK versions 49-52 on NPM and they all export requireNativeModule. Do we want to support SDKs lower than that?
I'll investigate more, thanks!
There was a problem hiding this comment.
Just got a great response from the Expo team on Discord! They confirmed we should import from "expo" instead of "expo-modules-core" since:
expo-modules-coreis meant to be internal to Expo's infrastructureexpopackage controls the correct version ofexpo-modules-core- We should set it up with:
- peerDependencies: expo: "^50 || ^51 || ^52" (for version ranges we want to support)
- devDependencies: expo: "" (for testing)
We're on the right track! 👍
There was a problem hiding this comment.
Curious, why there is still documentation that uses expo-modules-core directly.
- https://docs.expo.dev/modules/module-api/#function
- https://docs.expo.dev/modules/existing-library/#add-expo-packages-to-dependencies here they recommend
*for peer dep.
There was a problem hiding this comment.
Expo team confirmed it's an outdated doc 👍🏼
| "@clerk/types": "workspace:^" | ||
| }, | ||
| "devDependencies": { | ||
| "expo": "~52.0.0" | ||
| }, | ||
| "peerDependencies": { | ||
| "expo": "*", |
There was a problem hiding this comment.
Open for discussion regarding min version. requireNativeModule was only available since Expo SDK 49.
I think SDK 50 is good ^50 || ^51 || ^52
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change