feat: Add farcaster extension #750

ysm-dev · 2024-06-17T16:10:13Z

📦 Pull Request

[Provide a general summary of the pull request here.]

✅ Fixed Issues

[List any fixed issues here like: Fixes #XXXX]

🚨 Test instructions

[Describe any additional context required to test the PR/feature/bug fix.]

⚠️ Don't forget to add a semver label!

Please 🚨 ONLY ADD ONE 🚨 of the following labels, failing to do so may lead to adverse versioning of your changes when published:

patch: Bug Fix?
minor: New Feature?
major: Breaking Change?
skip-release: It's unnecessary to publish this change.

Special Note

Please avoid adding any of the Priority labels as they conflict with the labels above ☝️

📦 Published PR as canary version: Canary Versions

✨ Test out this PR locally via:

npm install @magic-ext/[email protected]
# or 
yarn add @magic-ext/[email protected]

packages/@magic-ext/farcaster/src/index.ts

Ethella · 2024-06-17T16:36:17Z

packages/@magic-ext/farcaster/src/index.ts

+
+ callback(data as any);
+
+ if (!showUI) return;


If showUI is false, will login the happen?

Yes, under this if statement is just for show QRCode in mandrake,
if showUI is false, we don't show our QRCode popup, developer should display their QRCode design.

A follow-up question, How would the developer know it's time to show the QRCode?

QRCode shows when dev calls magic.farcaster.login({ showUI: true })
So dev can add this function on Sign in with Farcaster Button onClick

Ethella · 2024-06-17T16:51:32Z

packages/@magic-ext/farcaster/src/index.ts

+ const handle = {
+ on: <T extends EventName>(event: T, callback: (params: EventMap[T]) => void) => {
+ if (event === EVENT.CHANNEL) {
+ !(async () => {


Why do we need an exclamation mark here?

Ethella · 2024-06-17T16:53:30Z

packages/@magic-sdk/types/src/modules/auth-types.ts

@@ -146,6 +146,10 @@ export enum AuthEventOnReceived {
 IDTokenCreated = 'Auth/id-token-created',
 }

+export enum FarcasterLoginEventEmit {
+ SuccessSignIn = 'success_sign_in',


Could we turn this to farcaster/success_sign_in

hcote · 2024-06-18T16:22:58Z

Is there a reason this logic can't be in the iframe? I think it's more maintainable that way where the sdk just sends an rpc method magic_auth_farcaster_login then the iframe handles the rest, then you don't have to pass events between the sdk and iframe.

Ethella · 2024-06-18T16:56:41Z

packages/@magic-ext/farcaster/src/index.ts

+ callback(data as any);
+ channel_token = data.channelToken;
+
+ if (isMobile()) {


I noticed a potential issue when the user is on a mobile device. The showUI condition won't have any effect in that case. If that's by design, I'd suggest putting a console warning or info to explain this. Otherwise, people might ask why the flag is not working.

yeah it's by design, we don't display QR on mobile device, and just open channel URL.
I will add console info

Ethella · 2024-06-18T17:06:39Z

scripts/bin/lint.ts

@@ -9,7 +9,7 @@ async function main() {
 const PKG = await promptForPackage();

 printSeparator('Linting TypeScripts');
- await execa('yarn', ['wsrun', '--serial', 'eslint', '--fix', '.'], {
+ await execa('yarn', ['wsrun', '--serial', 'eslint', '--fix'], {


Does this skip the linter stage?

I changed this to fix below typescript-eslint issue (explained below)

Ethella · 2024-06-18T17:33:28Z

package.json

@@ -69,7 +70,7 @@
 "ts-node": "^10.2.0",
 "tsc-watch": "^4.2.9",
 "tslib": "^2.3.1",
- "typescript": "~4.4.2",
+ "typescript": "^5.4.5",


I'd suggest not to bump the typescript version, as it might break other implementations. We'll need a thorough code scan once we bump the version

I bumped because @farcaster/auth-client library dependent to viem
and viem has peerDependency to TypeScript >5.0.4 https://github.com/wevm/viem/blob/main/src/package.json#L117
So it breaks CI when I installed viem in @magic-ext/farcaster

ysm-dev · 2024-06-20T14:45:29Z

Is there a reason this logic can't be in the iframe? I think it's more maintainable that way where the sdk just sends an rpc method magic_auth_farcaster_login then the iframe handles the rest, then you don't have to pass events between the sdk and iframe.

@hcote

When I initially designed the implementation, I thought that in the case where the user set showUI to false, there is no UI to show on the mandrake side at all, so it could only be handled in magic-js.

And to get both socket channel open event and success event from handler to farcaster login relayer, I thought it was similar because I need to send events from mandrake side to iframe side anyway.

hcote · 2024-06-20T16:15:43Z

We can handle this still in mandrake when there's no UI shown (this is how we handle ui-less requests like getInfo.

I think it'd be a lot simpler to maintain to have the logic in mandrake, and future updates would not require new sdk versions, but if there's a good reason why it needs to be in magic-js I'm open to doing it in here

Ethella · 2024-06-20T16:24:04Z

We can handle this still in mandrake when there's no UI shown (this is how we handle ui-less requests like getInfo.

I think it'd be a lot simpler to maintain to have the logic in mandrake, and future updates would not require new sdk versions, but if there's a good reason why it needs to be in magic-js I'm open to doing it in here

I believe the reason is that Farcaster, as a type of social login, may block authentication requests submitted from iframes or webviews in the future as a security measure, similar to Google OAuth.

hcote · 2024-06-20T16:26:07Z

packages/@magic-ext/farcaster/src/index.ts

+ channel_token = data.channelToken;
+
+ if (isMobile()) {
+ console.info(`Info: showUI parameter is ignored on mobile, open URL directly`);


This console.info should be removed

I added this from @Ethella 's review: #750 (comment)

Could you change the message to Magic -> farcaster extension: showUI parameter is ignored on mobile, open URL directly

hcote · 2024-06-20T16:28:04Z

@ysm-dev The deepsource errors are from the uses of any - they should be replaced with non-any types

Ethella · 2024-06-20T16:40:10Z

Created @magic-ext/farcaster package for CI publishing

Ethella · 2024-06-20T16:43:59Z

Please address the deepsource, otherwise LGTM

ysm-dev · 2024-06-20T17:13:20Z

@Ethella @hcote fixed!

svc-magic-git · 2024-06-20T18:59:37Z

🚀 PR was released in @magic-ext/[email protected] 🚀

feat: Add farcaster extension

ba9e2a1

ysm-dev requested review from Ethella and hcote June 17, 2024 16:10

ysm-dev self-assigned this Jun 17, 2024

ysm-dev requested a review from octave08 June 17, 2024 16:13

Ethella reviewed Jun 17, 2024

View reviewed changes

ysm-dev added 2 commits June 18, 2024 02:57

feat: Update FarcasterLoginEventEmit enum value to use namespace

866a300

feat: Update DEFAULT_TIMEOUT value in FarcasterExtension

9d09be8

ysm-dev requested a review from Ethella June 17, 2024 17:59

ysm-dev changed the base branch from master to chris/farcaster-types June 18, 2024 09:18

test

56435aa

Base automatically changed from chris/farcaster-types to master June 18, 2024 09:28

ysm-dev and others added 5 commits June 18, 2024 22:48

Merge branch 'master' into chris/farcaster-1

c93c764

mobile support

1b9a344

update lock

7f60c50

fix lock

bac6107

fix lint

dde0896

Ethella reviewed Jun 18, 2024

View reviewed changes

ysm-dev and others added 5 commits June 19, 2024 13:35

Merge branch 'master' into chris/farcaster-1

fa47cf6

fix lock

1e7272d

console info on mobile

915f930

console info

b0e88a6

fix

82b5b50

ysm-dev requested a review from Ethella June 19, 2024 06:02

fix

76ff11e

ysm-dev added 2 commits June 20, 2024 17:17

refactor

9efbb8e

fix

bf1f733

hcote reviewed Jun 20, 2024

View reviewed changes

ysm-dev added 2 commits June 21, 2024 01:55

Fix: deepSource

8375b77

Fix: deepSource

4387ff9

ysm-dev requested a review from hcote June 20, 2024 17:13

Ethella approved these changes Jun 20, 2024

View reviewed changes

ysm-dev added this pull request to the merge queue Jun 20, 2024

Merged via the queue into master with commit 99863af Jun 20, 2024
6 checks passed

ysm-dev deleted the chris/farcaster-1 branch June 20, 2024 18:36

svc-magic-git added the released This issue/pull request has been released. label Jun 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add farcaster extension #750

feat: Add farcaster extension #750

ysm-dev commented Jun 17, 2024 •

edited by svc-magic-git

Loading

Ethella Jun 17, 2024 •

edited

Loading

ysm-dev Jun 17, 2024

Ethella Jun 18, 2024

ysm-dev Jun 19, 2024

Ethella Jun 17, 2024

ysm-dev Jun 17, 2024

Ethella Jun 17, 2024

ysm-dev Jun 17, 2024

hcote commented Jun 18, 2024

Ethella Jun 18, 2024

ysm-dev Jun 19, 2024

Ethella Jun 18, 2024 •

edited

Loading

ysm-dev Jun 19, 2024

Ethella Jun 18, 2024

ysm-dev Jun 19, 2024

ysm-dev commented Jun 20, 2024

hcote commented Jun 20, 2024

Ethella commented Jun 20, 2024

hcote Jun 20, 2024

ysm-dev Jun 20, 2024

Ethella Jun 20, 2024

hcote commented Jun 20, 2024

Ethella commented Jun 20, 2024 •

edited

Loading

Ethella commented Jun 20, 2024

ysm-dev commented Jun 20, 2024

svc-magic-git commented Jun 20, 2024

feat: Add farcaster extension #750

feat: Add farcaster extension #750

Conversation

ysm-dev commented Jun 17, 2024 • edited by svc-magic-git Loading

📦 Pull Request

✅ Fixed Issues

🚨 Test instructions

⚠️ Don't forget to add a semver label!

Special Note

Ethella Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hcote commented Jun 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ethella Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ysm-dev commented Jun 20, 2024

hcote commented Jun 20, 2024

Ethella commented Jun 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hcote commented Jun 20, 2024

Ethella commented Jun 20, 2024 • edited Loading

Ethella commented Jun 20, 2024

ysm-dev commented Jun 20, 2024

svc-magic-git commented Jun 20, 2024

ysm-dev commented Jun 17, 2024 •

edited by svc-magic-git

Loading

Ethella Jun 17, 2024 •

edited

Loading

Ethella Jun 18, 2024 •

edited

Loading

Ethella commented Jun 20, 2024 •

edited

Loading