-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
deno_fetch attempt 2 #7524
deno_fetch attempt 2 #7524
Conversation
3468d0d
to
aec8c67
Compare
9168934
to
a83d692
Compare
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.
LGTM
Instead use Deno.core.jsonOpSync and Deno.core.jsonOpAsync
c93139f
to
7646747
Compare
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.
LGTM
We've been working on untangling fetch for many weeks - ultimately it required a bit of a sledge hammering to get it out. This patch is not the most elegant, but it does the job.
struct CreateHttpClientOptions { | ||
ca_file: Option<String>, | ||
} | ||
|
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 missing a --unstable check.
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.
There's no unstable check currently
Lines 174 to 191 in cead79f
fn op_create_http_client( | |
state: &mut OpState, | |
args: Value, | |
_zero_copy: &mut [ZeroCopyBuf], | |
) -> Result<Value, AnyError> { | |
let args: CreateHttpClientOptions = serde_json::from_value(args)?; | |
if let Some(ca_file) = args.ca_file.clone() { | |
super::cli_state(state).check_read(&PathBuf::from(ca_file))?; | |
} | |
let client = create_http_client(args.ca_file.as_deref()).unwrap(); | |
let rid = state | |
.resource_table | |
.add("httpClient", Box::new(HttpClientResource::new(client))); | |
Ok(json!(rid)) | |
} |
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.
Oh that is bad - there is meant to be one. Deno.createHttpClient()
is unstable.
Depends on #7521