-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
release(required): Parsing custom oAuth in amplify_outputs #13474
release(required): Parsing custom oAuth in amplify_outputs #13474
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just lint changes on this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
return providers.reduce<OAuthProvider[]>((oAuthProviders, provider) => { | ||
if (providerNames[provider] !== undefined) { | ||
oAuthProviders.push(providerNames[provider]); | ||
} | ||
|
||
return oAuthProviders; | ||
}, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially this?
return providers.reduce<OAuthProvider[]>((oAuthProviders, provider) => { | |
if (providerNames[provider] !== undefined) { | |
oAuthProviders.push(providerNames[provider]); | |
} | |
return oAuthProviders; | |
}, []); | |
return providers.map(provider => providerNames[provider]).filter(p => p); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes ! reduce reduces the two iteration :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤷
return providers.reduce<OAuthProvider[]>((oAuthProviders, provider) => { | ||
if (providerNames[provider] !== undefined) { | ||
oAuthProviders.push(providerNames[provider]); | ||
} | ||
|
||
return oAuthProviders; | ||
}, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can simplify this with filter
:
return providers.reduce<OAuthProvider[]>((oAuthProviders, provider) => { | |
if (providerNames[provider] !== undefined) { | |
oAuthProviders.push(providerNames[provider]); | |
} | |
return oAuthProviders; | |
}, []); | |
return providers.filter(provider => !!providerNames[provider]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slight note we would need map and filter in this case :P
* release(required): Parsing custom oAuth in amplify_outputs (#13474) * update parseAmplify logic * revert custom oAuth from gen1 config * update bundle size * chore(release): Publish [skip release] - @aws-amplify/adapter-nextjs@1.2.3 - @aws-amplify/analytics@7.0.35 - @aws-amplify/api@6.0.37 - @aws-amplify/api-graphql@4.1.6 - @aws-amplify/api-rest@4.0.35 - @aws-amplify/auth@6.3.5 - aws-amplify@6.3.6 - @aws-amplify/core@6.3.2 - @aws-amplify/datastore@5.0.37 - @aws-amplify/datastore-storage-adapter@2.1.37 - @aws-amplify/geo@3.0.35 - @aws-amplify/interactions@6.0.34 - @aws-amplify/notifications@2.0.35 - @aws-amplify/predictions@6.1.10 - @aws-amplify/pubsub@6.1.9 - @aws-amplify/storage@6.4.6 - tsc-compliance-test@0.1.39 * chore(release): update API docs [skip release] --------- Co-authored-by: ashika112 <155593080+ashika112@users.noreply.github.com> Co-authored-by: aws-amplify-bot <aws@amazon.com> Co-authored-by: Jim Blanchard <jim.l.blanchard@gmail.com>
Description of changes
The
parseAmplifyOutputs
function does not take into account any custom oAuth Provider and this lead to providers in oAuth config having undefined values breaking the UI Authenticator component.In additon to the fix for Amplify outputs, the PR loosens up the OAuth config type. Since looking into the issue, realized our parse utility provided for adding custom oAuth as string to list of providers outside of our
CustomProvider
inOAuthConfig
.Issue #, if available
#13466
Description of how you validated changes
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.