Skip to content

chore(flipt): swap underlying flipt web sdk #1244

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

Merged

Conversation

markphelps
Copy link
Contributor

@markphelps markphelps commented Mar 31, 2025

This PR

Closes: #1172

This updates the underlying Flipt Browser SDK to use our new 'isometric' JS SDK which supports both Node and Browser envs.

https://www.npmjs.com/package/@flipt-io/flipt-client-js

The older browser only sdk has been deprecated

Related Issues

Notes

Follow-up Tasks

How to test

@markphelps markphelps requested a review from a team as a code owner March 31, 2025 23:11
@lukas-reining
Copy link
Member

Sorry for the delay @markphelps, many of us have been at KubeCon last week.
Can you please sign off your commit? You can see how to do it here in the DCO error: https://github.com/open-feature/js-sdk-contrib/pull/1244/checks?check_run_id=39734812561

I will look what is wrong with the test :)

@markphelps markphelps force-pushed the update-flipt-web-underlying-sdk branch from 67da35e to 0355df0 Compare April 8, 2025 19:56
@markphelps
Copy link
Contributor Author

Sorry for the delay @markphelps, many of us have been at KubeCon last week. Can you please sign off your commit? You can see how to do it here in the DCO error: https://github.com/open-feature/js-sdk-contrib/pull/1244/checks?check_run_id=39734812561

I will look what is wrong with the test :)

Thanks @lukas-reining ! I just rebased to sign the DCO.

One issue I think is:

 libs/providers/flipt-web/src/lib/flipt-web-provider.ts:12:29 - error TS2307: Cannot find module '@flipt-io/flipt-client-js/browser' or its corresponding type declarations.

I'm importing @flipt-io/flipt-client-js/browser directly instead of the toplevel @flipt-io/flipt-client-js because the latter loads the correct implementation at runtime (nodejs vs browser), but I'd rather be extra safe here and just load the browser version since we know this SDK should only work in the browser.

I think its an issue with the linting tool in that it doesnt see @flipt-io/flipt-client-js/browser in the package.json file

@lukas-reining
Copy link
Member

lukas-reining commented Apr 9, 2025

Hey @markphelps, the way the repo currently works is, that most providers have their vendor SDK added as peerDependency in their package.json and the concrete version of it in the root package.json.
This is why the dependency is not installed when running the tests, as only the root dependencies are installed there.

Could you change this? We currently have @flipt-io/flipt-client-browser in the root package.json.
The same would be good for the flipt server provider then.

@markphelps markphelps requested a review from a team as a code owner April 10, 2025 14:16
@markphelps
Copy link
Contributor Author

@lukas-reining can I get some help fixing these linting issues? im not sure what it wants at this point

markphelps and others added 13 commits April 10, 2025 11:00
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Simon Schrottner <[email protected]>
Co-authored-by: Todd Baert <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps force-pushed the update-flipt-web-underlying-sdk branch from 4dc7e01 to f94c90c Compare April 10, 2025 15:00
@github-actions github-actions bot requested a review from adams85 April 10, 2025 15:00
@lukas-reining lukas-reining merged commit dba2a28 into open-feature:main Apr 10, 2025
7 checks passed
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