Skip to content

chore(expo-passkeys): Use Expo bundled version #5402

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

Merged
wobsoriano merged 6 commits into from
Mar 27, 2025
Merged

chore(expo-passkeys): Use Expo bundled version #5402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8f2522f
chore(clerk-expo): Bump expo-modules-core to 2.2.3 for Expo SDK 52 su…
wobsoriano Mar 20, 2025
4d90f5e
chore: add changeset
wobsoriano Mar 20, 2025
790d89e
chore(expo-passkeys): Remove expo-modules-core for future proofing
wobsoriano Mar 20, 2025
69e09a6
chore: dedupe
wobsoriano Mar 26, 2025
0d26730
chore: update changeset
wobsoriano Mar 26, 2025
b98ee95
chore: run dedupe
wobsoriano Mar 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/thick-rooms-repeat.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/expo-passkeys': minor
---

Removed `expo-modules-core` from dependencies in favor of `expo` as a peer dependency to allow broader compatibility with Expo SDK versions `^50 || ^51 || ^52`.
8 changes: 5 additions & 3 deletions packages/expo-passkeys/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@
},
"dependencies": {
"@clerk/shared": "workspace:^",
"@clerk/types": "workspace:^",
"expo-modules-core": "1.12.26"
"@clerk/types": "workspace:^"
},
"devDependencies": {
"expo": "~52.0.0"
},
"peerDependencies": {
"expo": "*",
"expo": "^50 || ^51 || ^52",
"react": "catalog:peer-react",
"react-native": "*"
}
Expand Down
2 changes: 1 addition & 1 deletion packages/expo-passkeys/src/ClerkExpoPasskeysModule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { requireNativeModule } from 'expo-modules-core';
import { requireNativeModule } from 'expo';

@panteliselef panteliselef Mar 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That would make the package compatible only with SDK 52, and we don't want that.

@wobsoriano wobsoriano Mar 20, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

Screenshot 2025-03-20 at 12 07 21 AM

What do you think?

@panteliselef panteliselef Mar 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think import { requireNativeModule } from 'expo'; is only available in SDK 52, at lease that's my understanding from their docs.

52

image

51

image

@wobsoriano wobsoriano Mar 20, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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!

Screenshot 2025-03-20 at 7 16 53 AM

@wobsoriano wobsoriano Mar 20, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just got a great response from the Expo team on Discord! They confirmed we should import from "expo" instead of "expo-modules-core" since:

  1. expo-modules-core is meant to be internal to Expo's infrastructure
  2. expo package controls the correct version of expo-modules-core
  3. 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! 👍

@panteliselef panteliselef Mar 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Curious, why there is still documentation that uses expo-modules-core directly.

@wobsoriano wobsoriano Mar 21, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Expo team confirmed it's an outdated doc 👍🏼

@wobsoriano wobsoriano Mar 26, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@panteliselef Opened the PR - our youtube partner followed up with the snapshot version they tried and said it worked for them 🙏🏼


// It loads the native module object from the JSI or falls back to
// the bridge module (from NativeModulesProxy) if the remote debugger is on.
Expand Down
Loading