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

Fixing issue with polyfill #961

Open
wants to merge 2 commits into
base: mainnet
Choose a base branch
from
Open

Conversation

Pauan
Copy link
Collaborator

@Pauan Pauan commented Jan 30, 2025

Fixes #956

Copy link
Collaborator

@iamalwaysuncomfortable iamalwaysuncomfortable left a comment

Choose a reason for hiding this comment

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

I figured out that most of the people speaking about this are operating in node.

Also, paraphrasing @onetrickwolf to provide context.

"I looked a little more into Issue 956. I thought this was a scoped export but I think since we are literally setting the globalThis.fetch it would replace anyone else's polyfill/fetch. I think the issue is, someone is importing the SDK into a nodejs project where they are already using a fetch polyfill for some other reason, then this overwrites it maybe breaking other functionality.

I think we did this as a fix just to quick fix all instances of fetch in the SDK that would want to read from file in nodejs, but I think we would need to do what @zkxuerb is saying: give it a custom name then replace all instances of fetch in the SDK with a customFetch. I think it's probably better practice to do that anyway, but I also don't know if we had to do this due to something in the wasm or something that we wouldn't be able to rename?"

@anthonyjoeseph
Copy link

anthonyjoeseph commented Feb 6, 2025

The polyfill breaks usage of the axios package as well. Not sure if this is already fixed by this PR but I wanted to mention just in case

import { ViewKey } from '@aleohq/sdk';
import axios from 'axios';

console.log(!!ViewKey);

axios.get('https://jsonplaceholder.typicode.com/todos/1').then((resp) => {
  console.log(resp.data);
});

output:

Error: Async XMLHttpRequest is not implemented yet

replit of this

@Pauan
Copy link
Collaborator Author

Pauan commented Feb 6, 2025

@anthonyjoeseph Thanks for bringing that to our attention, we're looking into it.

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.

Global fetch method impacts other SDKs
3 participants