Migrate the Dart Debug Extension to null-safety #1655
Conversation
annagrin
left a comment
annagrin
left a comment
There was a problem hiding this comment.
Thanks Elliott! some small comments...
| client.stream.listen((data) { | ||
| final message = serializers.deserialize(jsonDecode(data)); | ||
| if (message is ExtensionRequest) { | ||
| final messageParams = message.commandParams ?? ''; |
There was a problem hiding this comment.
Should this be '{}'? I think json.decode will throw on an empty string...
There was a problem hiding this comment.
Ah good point! Done
|
|
||
| @JS() | ||
| @anonymous | ||
| class ScriptIdParam { |
There was a problem hiding this comment.
Is this used? I couldn't find any explicit references in this file but maybe the use is implicit?
There was a problem hiding this comment.
You're right, I don't think this is being used anywhere. Removed
| external String get recipient; | ||
| external String get body; | ||
| external factory SimpleMessage({String recipient, String body}); | ||
| external factory SimpleMessage({String? recipient, String? body}); |
There was a problem hiding this comment.
Just curious - what happens if those parameters are non-null? Is it not possible to use them in interop then? Same question for other JS types we have in this file that we never pass nulls to.
There was a problem hiding this comment.
You mean null? Not sure, but I've switched to non-null since we always pass in a non null arg
There was a problem hiding this comment.
Sorry, I meant "can all the parameters be non-null?" For example, external factory SimpleMessage({String recipient, String body});. In this and some other external methods we never pass nulls. Does it not compile or fail at runtime?
There was a problem hiding this comment.
Made all parameters non-null if we don't pass in null values. Interestingly enough, this doesn't seem to have any effect on the dart2js compiled output.
2 participants
No description provided.