-
Notifications
You must be signed in to change notification settings - Fork 10
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
Update SSR instructions in README.md #66
Conversation
Use server-side SDK instead of client-side
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.
My understanding is that server-side code such as getServerSideProps should be using Unleash's server-side SDK, not the client-side SDK implemented in unleash-proxy-client... (right?)
It can use either Unleash Server Client or Unleash Frontend Client. Example above (SSG) is using Server Client (definitions). I like the idea of changing SSR example to the same code as SSG.
README.md
Outdated
|
||
return { | ||
props: { | ||
isEnabled: flags.isEnabled("nextjs-example"), | ||
// Or you can skip the flagsClient and access the toggles directly, but this will skip impression logging | ||
// and other client logic from unleash-proxy-client | ||
isEnabled: toggles.find((t) => t.name === 'FEATURE_NAME').enabled |
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.
This can be misleading. It's not a result of evaluation. This is from definitions. Feature can be enabled in definitions and still evaluate strategies to false
.
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.
Ooooh! Okay, I was confused about that - I'll just remove this...
This suggestion would not have properly evaluated the flags based on specified strategies.
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@unleash/nextjs](https://togithub.com/Unleash/unleash-client-nextjs) | [`1.4.1` -> `1.4.2`](https://renovatebot.com/diffs/npm/@unleash%2fnextjs/1.4.1/1.4.2) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@unleash%2fnextjs/1.4.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@unleash%2fnextjs/1.4.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@unleash%2fnextjs/1.4.1/1.4.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@unleash%2fnextjs/1.4.1/1.4.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>Unleash/unleash-client-nextjs (@​unleash/nextjs)</summary> ### [`v1.4.2`](https://togithub.com/Unleash/unleash-client-nextjs/releases/tag/v1.4.2) [Compare Source](https://togithub.com/Unleash/unleash-client-nextjs/compare/v1.4.1...v1.4.2) #### What's Changed - Update SSR instructions in README.md by [@​mltsy](https://togithub.com/mltsy) in [https://github.com/Unleash/unleash-client-nextjs/pull/66](https://togithub.com/Unleash/unleash-client-nextjs/pull/66) - fix: variant stickiness by [@​Tymek](https://togithub.com/Tymek) in [https://github.com/Unleash/unleash-client-nextjs/pull/70](https://togithub.com/Unleash/unleash-client-nextjs/pull/70) **Full Changelog**: Unleash/unleash-client-nextjs@v1.4.1...v1.4.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/simonknittel/simonknittel.de). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNjkuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCJ9-->
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@unleash/nextjs](https://togithub.com/Unleash/unleash-client-nextjs) | [`1.4.1` -> `1.4.2`](https://renovatebot.com/diffs/npm/@unleash%2fnextjs/1.4.1/1.4.2) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@unleash%2fnextjs/1.4.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@unleash%2fnextjs/1.4.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@unleash%2fnextjs/1.4.1/1.4.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@unleash%2fnextjs/1.4.1/1.4.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>Unleash/unleash-client-nextjs (@​unleash/nextjs)</summary> ### [`v1.4.2`](https://togithub.com/Unleash/unleash-client-nextjs/releases/tag/v1.4.2) [Compare Source](https://togithub.com/Unleash/unleash-client-nextjs/compare/v1.4.1...v1.4.2) #### What's Changed - Update SSR instructions in README.md by [@​mltsy](https://togithub.com/mltsy) in [https://github.com/Unleash/unleash-client-nextjs/pull/66](https://togithub.com/Unleash/unleash-client-nextjs/pull/66) - fix: variant stickiness by [@​Tymek](https://togithub.com/Tymek) in [https://github.com/Unleash/unleash-client-nextjs/pull/70](https://togithub.com/Unleash/unleash-client-nextjs/pull/70) **Full Changelog**: Unleash/unleash-client-nextjs@v1.4.1...v1.4.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/simonknittel/sinister-incorporated). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNjkuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Use server-side SDK instead of client-side
About the changes
My understanding is that server-side code such as getServerSideProps should be using Unleash's server-side SDK, not the client-side SDK implemented in unleash-proxy-client... (right?)
Important files
Discussion points
In reality, I think that this scenario should be leveraging the
unleash-client
Node SDK package, rather than any of the client-side types (i.e.IToggle
) or functionality provided by theunleash-proxy-client
frontend package. Which we could provide an example for here, but ideally this library should wrap that in something that defaults to using the same environment variables to configure a client, and maintain a singleton instance for you, etc.Using the Node client on the server-side would allow the app to have a singleton client instance that syncs itself with the Unleash server periodically, as recommended by the Unleash docs, and the user would only need to import that client and call
isEnabled
as needed, passing in a relevant context. I guess that's all being discussed in #45. For now, this PR at least demonstrates how to utilize the server-side SDK for server-side code.