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

fix: Add Stream Wrappers for use with various environments #4118

Merged
merged 14 commits into from
Jul 26, 2024

Conversation

RohinBhargava
Copy link
Member

@RohinBhargava RohinBhargava commented Jul 25, 2024

What's included:

  • Adds three new StreamWrapper implementations which injects the correct deps depending on environment:
  1. Node 18+ StreamWrapper that exposes methods for both fs.Writable and WritableStream
  2. Undici StreamWrapper that exposes methods for browser and cloudflare environments
  3. Node 16-17 StreamWrapper that exposes methods for fs.Writable
  4. Cloudflare CI Script that checks cloudflare can import SeedApiClient and instantiate it.
  5. Browser CI Script that checks browser can import SeedApiClient and instantiate it.

I know we discussed removing 16-17, but this would lead to a bigger change.

Testing on all environments using ElevenLabs and Vellum, will include source code for the tests

Notes:

More on CI tests, if there are bad imports, the CI checks expect to fail, when inspecting the stdout logs, e.g. for cloudflare:
Screenshot 2024-07-26 at 1 05 20 PM

FER-2497

@RohinBhargava RohinBhargava requested a review from dsinghvi as a code owner July 25, 2024 17:39
Copy link
Member

@dsinghvi dsinghvi left a comment

Choose a reason for hiding this comment

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

looks great

@RohinBhargava RohinBhargava changed the title wip: Add Stream Wrappers for use with various environments fix: Add Stream Wrappers for use with various environments Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants