Skip to content

Stop using dart:mirrors. #55

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

Merged
merged 1 commit into from
Feb 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.11.3+10

* Stop using `dart:mirrors`.

## 0.11.3+9

* Remove an extra newline in multipart chunks.
Expand Down
4 changes: 1 addition & 3 deletions lib/browser_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ import 'src/exception.dart';
import 'src/streamed_response.dart';

// TODO(nweiz): Move this under src/, re-export from lib/http.dart, and use this
// automatically from [new Client] once we can create an HttpRequest using
// mirrors on dart2js (issue 18541) and dart2js doesn't crash on pkg/collection
// (issue 18535).
// automatically from [new Client] once sdk#24581 is fixed.

/// A `dart:html`-based HTTP client that runs in the browser and is backed by
/// XMLHttpRequests.
Expand Down
6 changes: 1 addition & 5 deletions lib/src/client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import 'dart:typed_data';

import 'base_client.dart';
import 'base_request.dart';
import 'io.dart' as io;
import 'io_client.dart';
import 'response.dart';
import 'streamed_response.dart';
Expand All @@ -28,10 +27,7 @@ abstract class Client {
/// Currently this will create an [IOClient] if `dart:io` is available and
/// throw an [UnsupportedError] otherwise. In the future, it will create a
/// [BrowserClient] if `dart:html` is available.
Copy link
Member

Choose a reason for hiding this comment

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

This comment isn't relevant anymore, since you can't import this library without "dart:io" being available, can you?

Copy link
Member Author

Choose a reason for hiding this comment

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

It can now be importable while also being unavailable in the sense that its API just throws UnsupportedErrors.

factory Client() {
io.assertSupported("IOClient");
return new IOClient();
}
factory Client() => new IOClient();

/// Sends an HTTP HEAD request with the given headers to the given URL, which
/// can be a [Uri] or a [String].
Expand Down
55 changes: 0 additions & 55 deletions lib/src/io.dart

This file was deleted.

24 changes: 5 additions & 19 deletions lib/src/io_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,37 +3,24 @@
// BSD-style license that can be found in the LICENSE file.

import 'dart:async';
import 'dart:io';
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean "package:http/http.dart" can only be imported on the command line VM? If so, isn't that a breaking change?

Copy link
Member Author

Choose a reason for hiding this comment

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

As of 1.22, dart:io is importable on all platforms and will just throw UnsupportedErrors at runtime if it's not supported.

Copy link
Member

Choose a reason for hiding this comment

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

What?! Where was that announced? It's not in the CHANGELOG.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a @dgrove question.

Choose a reason for hiding this comment

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

Is the change to dart:io for 1.22 or 1.23? (I heard it was 1.23.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a fix out as #58


import 'package:async/async.dart';

import 'base_client.dart';
import 'base_request.dart';
import 'exception.dart';
import 'io.dart' as io;
import 'streamed_response.dart';

/// A `dart:io`-based HTTP client.
///
/// This is the default client when running on the command line.
class IOClient extends BaseClient {
/// The underlying `dart:io` HTTP client.
var _inner;
HttpClient _inner;

/// Creates a new HTTP client.
///
/// [innerClient] must be a `dart:io` HTTP client. If it's not passed, a
/// default one will be instantiated.
IOClient([innerClient]) {
io.assertSupported("IOClient");
if (innerClient != null) {
// TODO(nweiz): remove this assert when we can type [innerClient]
// properly.
assert(io.isHttpClient(innerClient));
_inner = innerClient;
} else {
_inner = io.newHttpClient();
}
}
IOClient([HttpClient inner]) : _inner = inner ?? new HttpClient();

/// Sends an HTTP request and asynchronously returns the response.
Future<StreamedResponse> send(BaseRequest request) async {
Expand Down Expand Up @@ -63,7 +50,7 @@ class IOClient extends BaseClient {
return new StreamedResponse(
DelegatingStream.typed/*<List<int>>*/(response).handleError((error) =>
throw new ClientException(error.message, error.uri),
test: (error) => io.isHttpException(error)),
test: (error) => error is HttpException),
response.statusCode,
contentLength: response.contentLength == -1
? null
Expand All @@ -73,8 +60,7 @@ class IOClient extends BaseClient {
isRedirect: response.isRedirect,
persistentConnection: response.persistentConnection,
reasonPhrase: response.reasonPhrase);
} catch (error) {
if (!io.isHttpException(error)) rethrow;
} on HttpException catch (error) {
throw new ClientException(error.message, error.uri);
}
}
Expand Down
8 changes: 4 additions & 4 deletions lib/src/multipart_file.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@

import 'dart:async';
import 'dart:convert';
import 'dart:io';

import 'package:async/async.dart';
import 'package:http_parser/http_parser.dart';
import 'package:path/path.dart' as path;

import 'byte_stream.dart';
import 'io.dart' as io;
import 'utils.dart';

/// A file to be uploaded as part of a [MultipartRequest]. This doesn't need to
Expand Down Expand Up @@ -85,12 +85,12 @@ class MultipartFile {
/// defaults to `application/octet-stream`, but in the future may be inferred
/// from [filename].
///
/// This can only be used in an environment that supports "dart:io".
/// Throws an [UnsupportedError] if `dart:io` isn't supported in this
/// environment.
static Future<MultipartFile> fromPath(String field, String filePath,
{String filename, MediaType contentType}) async {
io.assertSupported("MultipartFile.fromPath");
if (filename == null) filename = path.basename(filePath);
var file = io.newFile(filePath);
var file = new File(filePath);
var length = await file.length();
var stream = new ByteStream(DelegatingStream.typed(file.openRead()));
return new MultipartFile(field, stream, length,
Expand Down
4 changes: 2 additions & 2 deletions pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: http
version: 0.11.3+9
version: 0.11.3+10
author: "Dart Team <[email protected]>"
homepage: https://github.com/dart-lang/http
description: A composable, Future-based API for making HTTP requests.
Expand All @@ -12,4 +12,4 @@ dependencies:
dev_dependencies:
unittest: ">=0.9.0 <0.12.0"
environment:
sdk: ">=1.9.0 <2.0.0"
sdk: ">=1.22.0 <2.0.0"