-
Notifications
You must be signed in to change notification settings - Fork 52
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
Web3ApiClient - Preload API resolvers #777
Web3ApiClient - Preload API resolvers #777
Conversation
private deserializeOptions?: DeserializeManifestOptions, | ||
disablePreloadResolvers?: boolean | ||
) { | ||
if (disablePreloadResolvers) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a sort of hacky solution to the problem of using the aggregator in core-js tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm, should we use a TEST
environment variable maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but I think this is also has usecases elsewhere, eg a performance increase if you know all of your resolvers are local
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks really good!
I spent some time restructuring the terminology to something that I think makes more sense to new users. Would love to discuss here: #806
If you're okay with these proposed changes, I'm good with this PR :D
Everything looks great 💯
…te-recursion URI Resolver Terminology Updates
Closes: #715
Unblocks: polywrap/demos#23