Migrate frontend-server-common to null safety #1650
Conversation
|
I will delay this a bit until I make sure that frontend_server tests are running and passing: #1657 |
| @@ -327,5 +329,5 @@ String _parseBasePathFromIndexHtml(String index) { | |||
| } | |||
| final contents = file.readAsStringSync(); | |||
| final matches = RegExp(r'<base href="/([^>]*)/">').allMatches(contents); | |||
| return matches.isEmpty ? '' : matches.first.group(1); | |||
| return (matches.isEmpty ? null : matches.first.group(1)) ?? ''; | |||
There was a problem hiding this comment.
Why not matches.isEmpty ? '' : matches.first.group(1) ?
There was a problem hiding this comment.
first.group(1) may be null as well...
There was a problem hiding this comment.
Got it! I have an (optional suggestion) to switch this to be an if/else, the ternary and null-coalescing operator together is a bit hard to read.
| @@ -104,20 +101,20 @@ class StdoutHandler { | |||
| _badState = true; | |||
| return; | |||
| } | |||
| if (message.startsWith(boundaryKey)) { | |||
| if (message.startsWith(boundaryKey!)) { | |||
There was a problem hiding this comment.
Can we make boundaryKey non-nullable or handle it being null without the non null assert?
There was a problem hiding this comment.
Boundary key is set to null to indicate that the state machine is waiting for the new result from the frontend server (with a new boundary key) so I suspect we cannot make it non-nullable without changing the logic considerably. I will add a new non-null local to simply avoid repetition here.
Migrate frontend-server-common to null safety
Towards: #1327