Skip to content
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

Merged
merged 2 commits into from
Mar 7, 2024
Merged

Update SSR instructions in README.md #66

merged 2 commits into from
Mar 7, 2024

Conversation

mltsy
Copy link
Contributor

@mltsy mltsy commented Feb 21, 2024

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 the unleash-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.

Use server-side SDK instead of client-side
@thomasheartman
Copy link
Contributor

@mltsy Thanks for the suggestions here! I'm not familiar enough with Next.js to say for sure how we should do this. @Tymek: do you have any input here?

Copy link
Member

@Tymek Tymek left a 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
Copy link
Member

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.

Copy link
Contributor Author

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.
@mltsy mltsy requested a review from Tymek March 6, 2024 23:01
@Tymek Tymek merged commit d1a3ff2 into Unleash:main Mar 7, 2024
5 checks passed
renovate bot referenced this pull request in simonknittel/simonknittel.de Apr 2, 2024
[![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
(@&#8203;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
[@&#8203;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 [@&#8203;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-->
renovate bot referenced this pull request in simonknittel/sinister-incorporated Apr 2, 2024
[![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
(@&#8203;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
[@&#8203;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 [@&#8203;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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants