feat(space-nuxt-base): support app bridge #64
Conversation
eunjae-lee
changed the title
eunjae-lee
changed the title
| const appConfig = useAppConfig(); | ||
| const { initOAuth } = await readBody(event); | ||
| if (initOAuth) { | ||
| setCookie(event, AUTH_COOKIE_NAME, '', { |
There was a problem hiding this comment.
Is there a specific reason to set the cookie here ? I thought I would be set by the app-extension auth once it gets trough the flow.
There was a problem hiding this comment.
this is to clear the stale cookie in case a user re-installed the extension and needs to restart the whole oauth cookie. (do you remember the issue with the redirection the iframe)
There was a problem hiding this comment.
Oh yeah that makes sense. The question that I am asking myself/where I am torn, is if it makes more sense to somehow to handle this cookie reset inside app-extension-auth or in the backend, so that there are not many pieces floating around. Because when we decide to switch from the cookie approach and decide to store the cookie in a DB this behavior is loosely coupled with the rest so it might be harder to think of and consider then.
There was a problem hiding this comment.
Oh I see what you mean. Yeah it makes sense. Let's do the work on app-extension-auth to allow adaptors, and then move this logic back to app-extension-auth.
| }; | ||
| } | ||
|
|
||
| const appSession = await getAppSession(event); |
There was a problem hiding this comment.
Either here or inside getAppSession there should be new code to retrieve the session based on the result from session token.
| @@ -81,15 +101,6 @@ const useAppBridgeMessages = () => { | |||
| } | |||
| }; | |||
|
|
|||
| onUnmounted(() => { | |||
| // @ts-ignore not sure how to solve this | |||
| document.removeEventListener('message', eventListener); | |||
There was a problem hiding this comment.
I changed this to window.removeEventListener('message', eventListener); is this ok? Or do you think it will affect something it shouldnt @eunjae-lee
There was a problem hiding this comment.
oh, you're right. it's window.removeEventListener 🤦🏻♂️
|
I tested a little bit and there is an issue when reinstalling the plugin redirecting to 404. |
BibiSebi
force-pushed
the
feat/app-bridge
branch
from
e22458f
to
e888207
Compare
have you managed to fix it? or is it still a todo? |
|
Fixed the 401 issue, and also made some changes to initiate oauth outside the iframe. (refer to SHAPE-6131 ticket. It depends on a new change from Storyfront, which is in review at the moment) |
|
[NOTE] After merging this PR, we should update all the projects that rely on nuxt-base to have the v2 of app-extension-auth. Otherwise, the build will fail for those projects. It's the limitation from Nuxt that it doesn't install dependencies from the base layer properly for you. |
What?
This PR adds App Bridge support in nuxt-base project. It's turned off by default by
appConfig.appBridge.enabledbeingfalse. Users can turn it on on their project level.Why?
JIRA: SHAPE-5689
How to test? (optional)
https://www.loom.com/share/f3e501f581d844fcacac51681ca5e5ca