feat: add ability to define to redirect to app with different base path

[offgriddev]

Overview

This PR adds the ability to redirect to other app through the appPageURL and authPageURL config. On the client-side, when an AuthAction.REDIRECT_TO_LOGIN or AuthAction.REDIRECT_TO_APP are used, the current redirect logic always uses the next router, which only redirects within the current app. However, in micro-UI scenarios, these apps, while under the same domain, can be hosted in different apps with different base routes.

Currently, we can set these two URL config parameters as either strings or functions. By allowing users to assign an object to these configs, users can specify the metadata for the URL.

Changes

This PR adds support for this scenario by allowing the authPageURL and appPageURL to take object literals with the following schema:

{
   "destination": string,
   "basePath": boolean 
}

Due to the increased complexity of the shared redirect logic around locating the destination URL in withAuthUser and withAuthUserTokenSSR, I've extracted some common redirect functions in src/redirects. In doing so, I've unified the error messaging in the two components, which were just slightly different. I've also organized the component withAuthUser and separated the now more complex routing logic into a separate useCallback. The routing is done through window.location.assign if the user is being redirected to another app or router.route if the user is being redirect to the current app. The SSR component takes the whole object literal when assigned as it represents the SSR redirect object.

I've also cleaned up some documentation and added the favor around the new schema for authPageURL and appPageURL.

Checks

• Non-breaking API changes
• Tests

Issues

• closes Ability to ignore the basePath for authPageURL and appPageURL redirects at a global level #187

[vercel]

@offgriddev is attempting to deploy a commit to the Gladly Team on Vercel.

A member of the Team first needs to authorize it.

[prescottprue]

👍 Super helpful for us since it addresses #187

[codecov]

Codecov Report

Merging #352 (bb1185c) into main (0b76d60) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #352   +/-   ##
=======================================
  Coverage   99.27%   99.27%           
=======================================
  Files          25       26    +1     
  Lines         551      554    +3     
  Branches      202      188   -14     
=======================================
+ Hits          547      550    +3     
  Misses          4        4           

Impacted Files	Coverage Δ
src/redirects.js	100.00% <100.00%> (ø)
src/withAuthUser.js	100.00% <100.00%> (ø)
src/withAuthUserTokenSSR.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b76d60...bb1185c. Read the comment docs.

[offgriddev]

@kmjennison Hope you are well. I've reworked this PR to focus on resolving only issue #187. In the PR, I've allowed an object to be set or resolved to authPageURL or appPageURL that allows for redirecting to another app through a window.location.assign call instead of a call to the next router. I also included a focused refactor of some of the redirect code that I saw duplicated across the SSR and CSR code. One side effect to that is the error messaging changed slightly. The errors were slightly different in the two components, but I found the more generic messaged in the CSR component worked the best, so I kept that one. Let me know what you think or if you have any questions! 👋🏼

[kmjennison]

@offgriddev Thanks for this reworking!

One question I have is how Next.js handles the base path in its server-side redirect returned in getServerSideProps. I couldn't find documentation on this. Do you know if it always uses the base path, never uses the base path, or if that's configurable?

We'd want identical redirect URLs regardless of the redirect happening client-side or server-side. I haven't had a chance to look more closely, but it seems like they'd differ because the server-side redirect doesn't have any base path logic. Is that right?

[offgriddev]

One question I have is how Next.js handles the base path in its server-side redirect returned in getServerSideProps. I couldn't find documentation on this. Do you know if it always uses the base path, never uses the base path, or if that's configurable?

@kmjennison it actually looks like basePath is an undocumented property for redirect on getServerSideProps: https://github.com/vercel/next.js/blob/59f7676966570093f56ff35fb0d83eb97b394b7b/packages/next/server/base-server.ts#L1899

I’m going to look into this a bit more, but this looks to be the place where the redirect is handled on the server. If this is true, there is a little bit more I can do here in the SSR component.

[offgriddev]

@kmjennison Let me know what you think of the change I pushed. With the discovery of the basePath functionality implemented on the next server, the object literal for authPageURL and appPageURL could match the redirect object and be passed directly. To me, it cleans things up a bit and reduces the mapping, but let me know your thoughts!

[offgriddev]

@kmjennison NextJS responded to my issue quickly, and now the basePath is documented for SSR :)

[kmjennison]

Thanks for pushing this forward! It's great that Next supports what we need for this feature.

Requested changes. LMK if you have any questions.

[offgriddev]

@kmjennison Thanks for the review! I’ll get a revision up later today :)

[kmjennison]

With the greater flexibility in the authPageURL and appPageURL types, we'll also have to update the config section of the README and the TypeScript definitions.

[offgriddev]

@kmjennison 👋🏼 I hope your Monday is going well. I worked through your feedback. I just pushed some changes for your review when you get a chance.

In adding tests for the redirect module, I moved the logic for getting the item from the component config or the global config for the redirect. This was the code that looked like this const redirectDestination = appPageURL || getConfig().appPageURL. This allowed the caller to only need getRedirectToLoginDestination({ AuthUser, ctx }).

I also updated the TS and the README. Let me know if you want me to add any additional context there. Let me know if you'd like me to make any further adjustments!

[kmjennison]

@offgriddev Thanks for these changes. Another round of feedback. A few notes:

• Some feedback (esp for redirect logic) is opinionated about code clarity. If you'd prefer to ignore it, I can follow up with a PR to take care of those changes.
• Please push additional commits without squashing. That helps with code review on GitHub to compare what's changed since the last review.
• Nice work on the tests!

[offgriddev]

This logic really cleaned up

[offgriddev]

@kmjennison I just finished working through the feedback. Let me know if you have anything else, but I'm feeling good about these changes. Thanks for the helpful comments and suggestions on this PR!

I pushed up the commits as requested without squashing. Although, not sure how I managed to get yarn.lock changes here. I believe it happened after the last release, so probably user-error on my part.

[offgriddev]

@kmjennison I hope you're well. Is there any chance we can get this merged into the next release, or do you think there are some changes required still? Let me know. Happy Holidays to you if I don't hear back from you before next week.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/gladly-team/nfa-example/FescHTjWiz3NQ8YNxQoKFmRSbj9f
✅ Preview: https://nfa-example-git-fork-reside-eng-offgriddev-c-742a68-gladly-team.vercel.app

[kmjennison]

Looks great! @offgriddev thanks for the good work and iterations. I'll get this out in a release soon.

[offgriddev]

@kmjennison my pleasure! It was great to work with you through it!