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

docs: add tables for client-side sdks #3182

Closed
wants to merge 6 commits into from

Conversation

thomasheartman
Copy link
Contributor

@thomasheartman thomasheartman commented Feb 22, 2023

This PR adds an extra two tables to the SDK to describe client-side SDK capabilities

As requested in #3060.

What

  • Moves the server-side compat table under the server-side SDK header.
  • Adds two new tables under the client-side section:
    1. Client-side capabilities
    2. Proxy/edge/front-end api capabilities

Why

We've had the server-side overview for a good while now, but no equivalent for the client-side SDKs. This PR is an attempt to change that.

The client-side capabilites have been split into two tables because client-side capabilites depend on two variables: the SDK and what it connects to. As such, I have created a table for each.

@vercel
Copy link

vercel bot commented Feb 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 3:11pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ⬜️ Ignored (Inspect) Apr 26, 2023 3:11pm

@stale
Copy link

stale bot commented Mar 24, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 24, 2023
| **Category: Initialization** | | | | | | | |
| Async initialization | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ❌ |
| Can block until synchronized | ✅ | ✅ | ✅ | ⭕ | ⭕ | ✅ | ✅ |
| Default refresh interval | 10s | 15s | 15s | 15s | 15s | 30s | 30s |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g for Flutter it's 30 seconds for the refresh interval and the metrics interval. But maybe instead of syncing the table we should sync the SDKs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That we could, but it might also make sense for different SDKs to have different intervals. I'd be all for synchronizing the defaults, but I think we can do that as a separate thing.

| Default metrics interval | 60s | 60s | 60s | 60s | 60s | 60s | 30s |
| **Category: Custom Headers** | | | | | | | |
| static | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ |
| function | ✅ | ✅ | ⭕ | ✅ | ✅ (4.3) | ✅ | ✅ |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which SDK supports function based custom headers? What is the use case? Dynamic header names/values that change over time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today? Most of the server-side SDKs do:
image

That said, I don't know if any of the client-side SDKs do. But if they don't maybe we should make that clear.

| static | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ |
| function | ✅ | ✅ | ⭕ | ✅ | ✅ (4.3) | ✅ | ✅ |
| **Category: [Unleash Context](../reference/unleash-context)** | | | | | | | |
| Static fields (`environment`, `appName`) | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g. in flutter sdk we don't have environment field. You can add it in your custom fields but it's not built-in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's interesting! (And I think it's the right way to go). I think that this can be reworded a bit so that we can get away with it.

| Custom properties | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ |
| **Category: [Variants](../feature-toggle-variants.md)** | | | | | | | |
| Basic support | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ |
| [Custom stickiness (beta)](../stickiness.md#custom-stickiness-beta) | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense for the client side SDKs to be aware of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. That's one of the things I want to get feedback on. I think this might make more sense to have on the edge/proxy/feAPI table. I think that row can be moved.

| Basic usage metrics (yes/no) | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ |
| [Impression data](../impression-data.md) | ⭕ | ✅ | ⭕ | ⭕ | ⭕ | ⭕ | ✅ |
| **Category: Bootstrap (beta)** | | | | | | | |
| Bootstrap from file | ✅ | ✅ | ✅ | ⭕ | ✅ | ✅ | ✅ |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that bootstrap is just an object. You can read this object from a file but it doesn't seem to be Unleash client responsibility to reach out to a file. I'd rather pipe the output of a file to Unleash client (Unix philosophy)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair assessment, but we have built that into at least a few of the server-side SDKs, which is why it's in the table. I don't know if any of the client-side clients support this at the moment (unlikely because they're generally not on systems where you have file access, right?), so we could potentially get rid of it, yea.

@stale
Copy link

stale bot commented May 26, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 26, 2023
@stale stale bot closed this Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: create compatibility matrix for client-side SDKs
2 participants