Enable null safety #472

kevmoo · 2020-09-24T17:29:54Z

No description provided.

kevmoo · 2020-09-24T17:30:20Z

Sadly, test/streamed_request_test.dart is failing on safari – but all of the other tests are passing

lib/src/io_streamed_response.dart

kevmoo · 2020-09-24T17:36:13Z

lib/src/utils.dart

  var pairs = <List<String>>[];
  map.forEach((key, value) => pairs.add([
-        Uri.encodeQueryComponent(key, encoding: encoding),
-        Uri.encodeQueryComponent(value, encoding: encoding)
+        Uri.encodeQueryComponent(key, encoding: encoding ?? utf8),


Uri.encodeQueryComponent encoding param is now non-null with a default.

So we have to hoist the default value here for a null encoding

kevmoo · 2020-09-24T17:36:53Z

lib/src/browser_client.dart

    unawaited(xhr.onLoad.first.then((_) {
-      // TODO(nweiz): Set the response type to "arraybuffer" when issue 18542


This issue is now closed

kevmoo · 2020-09-24T17:37:16Z

lib/src/browser_client.dart

-      // TODO(nweiz): Set the response type to "arraybuffer" when issue 18542
-      // is fixed.
-      var blob = xhr.response as Blob ?? Blob([]);
+      var blob = xhr.response as Blob;


According to the annotations on the dart:html types, response is never null.

What do you mean by this? I think it probably is true that if onLoad.first doesn't have an error this would be non-null, but I don't know what you mean by "the annotations on the dart:html types".

kevmoo · 2020-09-24T17:37:51Z

lib/src/browser_client.dart

@@ -52,16 +52,17 @@ class BrowserClient extends BaseClient {
    request.headers.forEach(xhr.setRequestHeader);

    var completer = Completer<StreamedResponse>();
+
+    // TODO(kevmoo): not sure what to do here...
+    // ignore: void_checks


Here and below – not sure why we're getting this void_checks hint...

This will be worked around by googlearchive/pedantic#69 if we get that in.

This issue looks like https://github.com/dart-lang/linter/issues/2185

I think we can keep the lint and point the todo at that issue.

lib/src/base_request.dart

lib/src/io_client.dart

pubspec.yaml

jakemac53 · 2020-09-24T18:12:59Z

pubspec.yaml

@@ -1,15 +1,19 @@
 name: http
-version: 0.12.2
+version: 0.12.3-nullsafety


I think we are generally going with breaking changes for non-allow-listed packages, although that isn't totally decided

Oh it sounds like from the changelog entry this is in the allow list, even so we really only don't want to do breaking changes for flutter depended on packages.

Flutter doesn't depend on this directly – at least I don't think so.

A lot of stuff in the Flutter ecosystem does, though...

cc @natebosch I would still do a major version bump.

We need to stop setting a bad example here.

Oh it sounds like from the changelog entry this is in the allow list

It isn't.

I would still do a major version bump.

+1. We'll want a breaking change for #424 in any case. I can try to get to that next week so we can make a decision before we need to publish this.

kevmoo · 2020-09-25T18:42:31Z

where are we on this review, kind sirs?

CHANGELOG.md

natebosch · 2020-09-25T21:39:49Z

analysis_options.yaml

 linter:
  rules:
  - annotate_overrides
  - avoid_bool_literals_in_conditional_expressions
+  - avoid_catching_errors


Enabling lints is not related to null safety and should go in a separate PR.

natebosch · 2020-09-25T21:42:47Z

lib/src/boundary_characters.dart

@@ -10,7 +10,7 @@
 ///
 /// [RFC 2046]: http://tools.ietf.org/html/rfc2046#section-5.1.1.
 /// [RFC 1521]: https://tools.ietf.org/html/rfc1521#section-4
-const List<int> BOUNDARY_CHARACTERS = <int>[
+const List<int> boundaryCharacters = <int>[


use a separate PR.

natebosch · 2020-09-25T22:09:11Z

lib/src/browser_client.dart

@@ -52,16 +52,17 @@ class BrowserClient extends BaseClient {
    request.headers.forEach(xhr.setRequestHeader);

    var completer = Completer<StreamedResponse>();
+
+    // TODO(kevmoo): not sure what to do here...
+    // ignore: void_checks


This will be worked around by googlearchive/pedantic#69 if we get that in.

This issue looks like https://github.com/dart-lang/linter/issues/2185

I think we can keep the lint and point the todo at that issue.

natebosch · 2020-09-25T22:13:06Z

lib/src/browser_client.dart

-      // TODO(nweiz): Set the response type to "arraybuffer" when issue 18542
-      // is fixed.
-      var blob = xhr.response as Blob ?? Blob([]);
+      var blob = xhr.response as Blob;


What do you mean by this? I think it probably is true that if onLoad.first doesn't have an error this would be non-null, but I don't know what you mean by "the annotations on the dart:html types".

pubspec.yaml

kevmoo · 2020-09-29T03:29:46Z

PTAL – cleaned things up

jakemac53 · 2020-09-29T15:40:53Z

Please don't force push because now we can no longer see what you changed compared to our last review :(

kevmoo · 2020-09-29T15:42:50Z

Gah. Right. I wanted to revert a bunch of stuff and that seemed easiest.

…

On Tue, Sep 29, 2020 at 8:41 AM Jacob MacDonald ***@***.***> wrote: Please don't force push because now we can no longer see what you changed compared to our last review :( — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#472 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAEFCXWVTYLQVINIONLIVDSIH5ZLANCNFSM4RYSLUFQ> .

jhancock4d · 2021-01-04T23:28:08Z

Is there a beta package with this enabled available? I can't find it and this is blocking in almost any project to being able to safely turn on null safety.

natebosch · 2021-01-14T20:09:17Z

Publishing is tracked in #501 (comment)

kevmoo requested review from jakemac53 and natebosch September 24, 2020 17:29

googlebot added the cla: yes label Sep 24, 2020

kevmoo commented Sep 24, 2020

View reviewed changes

lib/src/io_streamed_response.dart Show resolved Hide resolved

kevmoo commented Sep 24, 2020

View reviewed changes

lib/src/base_request.dart Outdated Show resolved Hide resolved

jakemac53 reviewed Sep 24, 2020

View reviewed changes

lib/src/base_request.dart Show resolved Hide resolved

jakemac53 reviewed Sep 24, 2020

View reviewed changes

lib/src/io_client.dart Show resolved Hide resolved

jakemac53 reviewed Sep 24, 2020

View reviewed changes

pubspec.yaml Show resolved Hide resolved

jakemac53 reviewed Sep 24, 2020

View reviewed changes

natebosch reviewed Sep 25, 2020

View reviewed changes

Enable null safety

ebed263

kevmoo force-pushed the null_safety branch from 99553ea to ebed263 Compare September 29, 2020 02:35

jakemac53 approved these changes Sep 29, 2020

View reviewed changes

kevmoo merged commit 20e3a59 into master Sep 29, 2020

kevmoo deleted the null_safety branch September 29, 2020 16:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable null safety #472

Enable null safety #472

kevmoo commented Sep 24, 2020

kevmoo commented Sep 24, 2020

kevmoo Sep 24, 2020

kevmoo Sep 24, 2020

kevmoo Sep 24, 2020

natebosch Sep 25, 2020

kevmoo Sep 24, 2020

natebosch Sep 25, 2020

jakemac53 Sep 24, 2020

jakemac53 Sep 25, 2020

kevmoo Sep 25, 2020

jakemac53 Sep 25, 2020

natebosch Sep 25, 2020

kevmoo commented Sep 25, 2020

natebosch Sep 25, 2020

natebosch Sep 25, 2020

natebosch Sep 25, 2020

natebosch Sep 25, 2020

kevmoo commented Sep 29, 2020

jakemac53 commented Sep 29, 2020

kevmoo commented Sep 29, 2020 via email

jhancock4d commented Jan 4, 2021

natebosch commented Jan 14, 2021

		unawaited(xhr.onLoad.first.then((_) {
		// TODO(nweiz): Set the response type to "arraybuffer" when issue 18542

Enable null safety #472

Enable null safety #472

Conversation

kevmoo commented Sep 24, 2020

kevmoo commented Sep 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevmoo commented Sep 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevmoo commented Sep 29, 2020

jakemac53 commented Sep 29, 2020

kevmoo commented Sep 29, 2020 via email

jhancock4d commented Jan 4, 2021

natebosch commented Jan 14, 2021