Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: express example #50
feat: express example #50
Changes from 25 commits
f288c4b
4da9c8d
e01e1f6
548b862
7736b05
a620203
d2b4a89
8f8d830
e3a68f0
897e57f
620a0f8
af44a5c
f2d9bdc
e030880
fbc0654
0752907
dfc592b
bc86541
d440639
c6d3c08
4e0c566
fe0236b
306ac8d
9b6f35a
ca6c50a
bf5470a
930c290
aca2c32
a0992a4
f17059f
beef837
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
do we need to add instructions here on how to run the frontend?
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.
We already have it in our docs (https://docs.id.gov.sg/integrations-with-sgid/typescript-javascript/express-with-single-page-app-frontend#step-5-test-it-out) but let me add it here too
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.
updated in ca6c50a
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.
nit: should we destructure the
nonce
andcodeVerifier
from the session? because in/userinfo
the values are being extracted from the session before being passed touserinfo
(might also make passing the params a bit neater)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.
fixed it in the callback code in aca2c32
i didn't fix it for userinfo because you can't destructure an undefined item (
session
is possibly undefined). We could check for whethersession
is defined first, but since we also need to check for access token and sub, it doesn't really make sense to destructure there