Skip to content

Adds Flagsmith adapter #103

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

tiagoapolo
Copy link

@tiagoapolo tiagoapolo commented Mar 13, 2025

This pull request introduces a new provider adapter for the Flags SDK that integrates with Flagsmith. The changes include adding configuration files, updating dependencies, and creating the necessary code for the adapter.

Flagsmith documentation: https://docs.flagsmith.com/clients/next-ssr

Usage

import { createFlagsmithAdapter } from '@flags-sdk/adapter-flagsmith';
import { flag } from 'flags';

// Create the Flagsmith adapter
const flagsmithAdapter = createFlagsmithAdapter({
  environmentID: 'your-environment-id',
  // Optional: Add any other Flagsmith configuration options
  // See: https://docs.flagsmith.com/clients/javascript/
});

// Define your flags
const myFeatureFlag = flag({
  key: 'my-feature',
  adapter: flagsmithAdapter,
});

// Use the flag in your application
const flag = await myFeatureFlag();

Copy link

vercel bot commented Mar 13, 2025

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

A member of the Team first needs to authorize it.

Copy link

socket-security bot commented Mar 13, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub ↗.

Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added[email protected]1001008097100

View full report ↗

@tiagoapolo tiagoapolo marked this pull request as ready for review March 14, 2025 14:08
@tiagoapolo tiagoapolo marked this pull request as draft March 14, 2025 14:08
@tiagoapolo tiagoapolo marked this pull request as ready for review March 14, 2025 14:11
Copy link
Collaborator

@dferber90 dferber90 left a comment

Choose a reason for hiding this comment

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

Thank you for your contributions so far! I left a bit of early guidance

@@ -0,0 +1,64 @@
# Flags SDK - Flagsmith Provider

A provider adapter for the Flags SDK that integrates with [Flagsmith](https://flagsmith.com/), allowing you to use Flagsmith's feature flags and remote configuration in your application.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you work for Flagsmith? If we merge this PR we'd also put up some actual docs on flags-sdk.dev. Would you be wiling to link to our Flags SDK and those docs from https://docs.flagsmith.com?

Choose a reason for hiding this comment

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

Hey @dferber90, just jumping in here since Tiago is on vacation - yes, Tiago works for Flagsmith.

In answer to your question, for sure, we'd be happy to link to your docs from https://docs.flagsmith.com - would you be happy with adding reciprocal backlink(s) too?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great! We'd add you to https://flags-sdk.dev/providers and create a docs page for FlagSmith

@tiagoapolo
Copy link
Author

@dferber90 could you take one more look at my PR ?

@dferber90
Copy link
Collaborator

@tiagoapolo I‘m out of office this week, but @AAorris will take over leading adapter work and help you get this finalized, listed and released 👍

Copy link
Contributor

@AAorris AAorris left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! Please consider the following input:

Strongly recommended:

  • Support identity through entities
  • Optimize performance for multiple features in one request

Recommended:

  • Add origin to link flags to your customers' dashboards

defaultValue,
}): Promise<ValueType> {
await initialize(configParams);
const state = flagsmith.getState();
Copy link
Contributor

@AAorris AAorris Apr 10, 2025

Choose a reason for hiding this comment

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

Note that if there are multiple flags, getState() might be called many times in one request:

a = flag({ key: 'a', adapter: flagSmith.getFeature() })
b = flag({ key: 'a', adapter: flagSmith.getFeature() })
...
await Promise.all([a(), b()])

You may consider ways to reduce duplicate work if it's not deduplicated in your SDK

return {
async decide({
key,
entities,
Copy link
Contributor

Choose a reason for hiding this comment

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

Entities is unused right now, but it's where Flags SDK usually receives user information. I'd expect identity to be passed to flagsmith.init({}) here if that's how your SDK works
https://docs.flagsmith.com/clients/next-ssr#comparing-ssr-and-client-side-flagsmith-usage

If you expect your customers to pass an identity, you should strongly consider using entities to drive that here.

What I'd expect is something like:

import type { Identity } from 'flagsmith'

// ...
ValueType extends IFlagsmithFeature,
EntitiesType extends IFlagsmithIdentity,

// ...
decide({ entities }) => {
  // identity is either entities itself, or a part of it
  init({ identity: entities })
}

Comment on lines +15 to +18
async function initialize(config: IInitConfig) {
if (flagsmith?.initialised) return;
await flagsmith.init({ fetch: global.fetch, ...configParams });
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like flagsmith may be initialized with a user — In that case, this pattern won't work because we'll never re-initialize for another user. I'll need to better understand how identity fits in here, and how it will handle the case where there's multiple different users depending on the request.

https://github.com/Flagsmith/flagsmith-js-examples/blob/main/nextjs-middleware/middleware.ts#L15

Comment on lines +20 to +21
return {
async decide({
Copy link
Contributor

Choose a reason for hiding this comment

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

Adapters can provide origin here, which is a hyperlink to the feature flag defined in code. If you can provide this hyperlink, it will allow users to get to the right place in one click from the Flags Explorer

https://flags-sdk.dev/frameworks/next#flags-explorer

Ex. something like

return { 
  origin: `${configParams.api}/${configParams.environmentId}/feature/${key}`
  decide,
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants