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

Consider lazy loading ky-universal #34

Closed
dlongley opened this issue Mar 27, 2023 · 1 comment · Fixed by #35
Closed

Consider lazy loading ky-universal #34

dlongley opened this issue Mar 27, 2023 · 1 comment · Fixed by #35

Comments

@dlongley
Copy link
Member

dlongley commented Mar 27, 2023

See: digitalbazaar/jsonld.js#516 (comment)

@Peeja,

Thanks, great analysis!

It seems that a PR to http-client could be made that did something to the internals ... like making _createHttpClient take a function that returned a (possibly memoized) kyPromise rather than taking a promise directly. That would allow the original import of ky-universal to also be moved within that function for the default case where the parent is the original / base ky instance:

https://github.com/digitalbazaar/http-client/blob/v3.3.0/lib/httpClient.js#L35-L51

Unfortunately, it seems this would mean that the kyPromise exported by the library would also need to become such a function, which would be a breaking change. Maybe we can figure out a way around that -- but even if we can't, a major version could be released that avoids the problem you identified.

Any further discussion that relates to http-client specifically should be taken over here: digitalbazaar/http-client#34

@dlongley
Copy link
Member Author

I'm not sure we intended for kyPromise to be a used export (though we didn't underscore-prefix it) ... maybe we exposed it for internal / testing usage only?

Perhaps we'd just make it so that the exported kyPromise would never resolve until some call was made to import ky-universal... which is already true, but we currently always make that call. So what would be changing would be when / what triggers the resolution of that Promise. We may be able to get away with that as a non-breaking change.

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 a pull request may close this issue.

1 participant