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

Polyfill AbortController, AbortSignal #322

Open
radu-matei opened this issue Feb 1, 2025 · 10 comments
Open

Polyfill AbortController, AbortSignal #322

radu-matei opened this issue Feb 1, 2025 · 10 comments

Comments

@radu-matei
Copy link
Member

ref bytecodealliance/StarlingMonkey#154

@talentlessguy
Copy link

there's no need to load polyfills in the library itself. You can load them globally in your project like this:

if (!globalThis.AbortController) globalThis.AbortController = AbortController

@karthik2804
Copy link
Collaborator

@talentlessguy I agree with you that the user can use the polyfill on an as-needed basis. I have found that abortcontroller-polyfill seems to do the required job when I try to use libraries like @langchain/ollama.

@radu-matei
Copy link
Member Author

Hey, @talentlessguy, thanks for chiming in!

Clarifying here a bit -- does this mean users would have to add the snippet above in their projects? Or you imagine the templates adding that by default? Or adding it as part of the SDK?

@talentlessguy
Copy link

Hey, @talentlessguy, thanks for chiming in!

Clarifying here a bit -- does this mean users would have to add the snippet above in their projects? Or you imagine the templates adding that by default? Or adding it as part of the SDK?

consumers of the SDK would have to add it in their code somewhere, they way most polyfills are loaded

@ThorstenHans
Copy link
Contributor

@karthik2804 and I were talking about this as well.

IMO, users should not have to deal with this. Instead, we could use webpack to add the necessary polyfills automatically.

@talentlessguy
Copy link

talentlessguy commented Feb 9, 2025

@ThorstenHans if a user is using a platform where AC is not supported, they would inevitably have to load the polyfill themselves anyway since Fastly does not support those and a bunch of other packages rely on it's global availability.

In the end you end up with

  1. duplicate polyfills, oftentimes people install 2 different packages (there's 3 competing polyfills for AC right now) for polyfilling the same functionality and you end up loading multiple implementations, increasing memory usage of the program
  2. all current polyfills only implement the basic functionality and don't follow the spec closely
  3. unnecessary dependency, yet another supply chain risk which is already pretty difficult to manage

polyfills were never meant to be loaded in a library, they should be configured through your bundler/global setup script

@radu-matei
Copy link
Member Author

Ok, so if I'm reading this correctly, it would be ok to load it in the webpack config, but not bundle it with the SDK?

@karthik2804
Copy link
Collaborator

karthik2804 commented Feb 9, 2025

@radu-matei , yes partially - it should not included with the SDK. The question is if we add it by default in the webpack config or let the user add it to webpack as and when needed until it is implemented upstream in StarlingMonkey. I am slightly preferential to letting the user add it if their program or choice of library requires it (we should document it as a possibly solution). We should look into adding a readme into the templates that can help out users in situations like this.

@ThorstenHans
Copy link
Contributor

Would it have a negative effect if it's added to the default webpack config? If not I'd prefer this approach.

@talentlessguy
Copy link

Ok, so if I'm reading this correctly, it would be ok to load it in the webpack config, but not bundle it with the SDK?

yes that's okay, sorry i thought you meant webpack as the library's bundler

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

No branches or pull requests

4 participants