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

Add a dart:html client #83

Merged
merged 4 commits into from
May 25, 2017
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions lib/browser_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import 'dart:typed_data';
import 'package:stack_trace/stack_trace.dart';

import 'src/base_client.dart';
import 'src/base_request.dart';
import 'src/byte_stream.dart';
import 'src/exception.dart';
import 'src/streamed_response.dart';
import 'src/request.dart';
import 'src/response.dart';

// TODO(nweiz): Move this under src/, re-export from lib/http.dart, and use this
// automatically from [new Client] once sdk#24581 is fixed.
Expand Down Expand Up @@ -41,16 +41,17 @@ class BrowserClient extends BaseClient {
bool withCredentials = false;

/// Sends an HTTP request and asynchronously returns the response.
Future<StreamedResponse> send(BaseRequest request) async {
var bytes = await request.finalize().toBytes();
Future<Response> send(Request request) async {
var stream = new ByteStream(request.read());
Copy link
Member

Choose a reason for hiding this comment

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

I really want to kill ByteStream, so we shouldn't use it here. You can pull this functionality into a function in utils/, although really it should live in a package somewhere.

Copy link
Author

Choose a reason for hiding this comment

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

It appears it IS in a package 👍

var bytes = await stream.toBytes();
var xhr = new HttpRequest();
_xhrs.add(xhr);
_openHttpRequest(xhr, request.method, request.url.toString(), asynch: true);
xhr.responseType = 'blob';
xhr.withCredentials = withCredentials;
request.headers.forEach(xhr.setRequestHeader);

var completer = new Completer<StreamedResponse>();
var completer = new Completer<Response>();
xhr.onLoad.first.then((_) {
// TODO(nweiz): Set the response type to "arraybuffer" when issue 18542
// is fixed.
Expand All @@ -59,13 +60,12 @@ class BrowserClient extends BaseClient {

reader.onLoad.first.then((_) {
var body = reader.result as Uint8List;
completer.complete(new StreamedResponse(
new ByteStream.fromBytes(body),
completer.complete(new Response(
xhr.responseUrl,
xhr.status,
contentLength: body.length,
request: request,
headers: xhr.responseHeaders,
reasonPhrase: xhr.statusText));
reasonPhrase: xhr.statusText,
body: new ByteStream.fromBytes(body),
Copy link
Member

Choose a reason for hiding this comment

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

This also shouldn't be a ByteStream. In this case, just new Stream.fromIterable([body]) should work.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

headers: xhr.responseHeaders));
});

reader.onError.first.then((error) {
Expand Down