Support null safety

[al8n]

Thanks for your amazing package. Is there a plan to support the null-safety version?

[sowens-csd]

Thanks for the positive feedback, I appreciate it!

Yes, I'll definitely be updating it for null-safety. I don't have a schedule for that work yet but it will begin soon.

[kaladron]

I started on this for fun this morning - How are the .g. files generated?

[sowens-csd]

First of all, you have a strange definition of fun, but thank you! I'll have a look at your PRs but probably won't merge until after I've done the first release with web support. I really appreciate the help.

The .g files are built using the flutter pub run build_runner build command. Sometimes it needs the delete conflicts option to force it through.

[kaladron]

This isn't the weirdest open source task I've taken on =)

I've been planning to do one PR per repository if that's OK, that way the review can be done in smaller chunks.

It seems like it's traditional to do a major release bump for null-safety and people use that as a chance to clean up the API, so it makes sense to hold it off. It might be worth merging to a branch if you have other interface changes you'd like to do.

Whatever way you'd like this, please let me know!

[sowens-csd]

Yes, one per repo would be great. It will be a major release and I think we should do it on a branch. That way I won't have to delay merging your PRs. I've just created a null-safety branch from main. Let me know if that works for you.

[kaladron]

That's perfect. I'm still learning GitHub (I've used it for solo projects, but not much for collaboration) - if I need to send the PRs in a different way to target the branch, please let me know. I'll make sure I rebase my changes to the branch. Thanks!

[kaladron]

I've rebased my changes onto the branch now and targeted the PR at the branch. Thanks!

[kaladron]

Hi - sorry for the newb question: How do you tell the code to use the library in the sibling directory instead of in the dart pub system? When I'm building the speech_to_text, I don't know how to tell it to use my platform_interface instead of yours.

Thanks!

[sowens-csd]

You can use a path reference. It looks like this:

dependencies:
  speech_to_text_platform_interface:
    path: ../speech_to_text_platform_interface/

Sorry I've been absent on this issue, been AFK for a bit. I'm back now and I'll have a look at your PR.

[kaladron]

Thanks, I'll try to get to the other one this evening, but might be tomorrow.

On Sun, Mar 7, 2021, 12:24 Stephen Owens notifications@github.com wrote:

You can use a path reference. It looks like this:

dependencies: speech_to_text_platform_interface: path: ../speech_to_text_platform_interface/

Sorry I've been absent on this issue, been AFK for a bit. I'm back now and I'll have a look at your PR.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/csdcorp/speech_to_text/issues/164#issuecomment-792346040, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4FOZRXMIQLHF5H5GSTBLTCPOI5ANCNFSM4V7NMRSQ .

[sowens-csd]

Question for you, in the MethodChannelSpeechToText class you've used this syntax to match null safety requirements:

    var result = await _channel.invokeMethod<bool>('has_permission');

    return result!;

I don't love the intermediate variable although you're right that appears to be the only clean way to make the syntax work. There's an alternative form, which is this:

    return await _channel.invokeMethod<bool>('has_permission') ?? false;

Do you have a preference between the two forms? You may have already thought this through in choosing the syntax you did so let me know.

[kaladron]

I don't have a preference. I wasn't confident that the await would chain right to the ?? operator. (I still have trouble reasoning futures sometimes). If it works, ?? Is better than a test because the variable is never in there intermediate null state.

On Sun, Mar 7, 2021, 13:11 Stephen Owens notifications@github.com wrote:

Question for you, in the MethodChannelSpeechToText class you've used this syntax to match null safety requirements:

var result = await _channel.invokeMethod<bool>('has_permission');

return result!;

I don't love the intermediate variable although you're right that appears to be the only clean way to make the syntax work. There's an alternative form, which is this:

return await _channel.invokeMethod<bool>('has_permission') ?? false;

Do you have a preference between the two forms? You may have already thought this through in choosing the syntax you did so let me know.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/csdcorp/speech_to_text/issues/164#issuecomment-792353814, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4FO7DWPFRVQ5RNKMADNLTCPTWHANCNFSM4V7NMRSQ .

[kaladron]

Oh, I also filed an issue about how I don't think this can ever be null. I realized yesterday that I filled it under web instead off on the api.

https://github.com/flutter/website/issues/5297

On Sun, Mar 7, 2021, 13:41 Jeff Bailey jbailey@raspberryginger.com wrote:

I don't have a preference. I wasn't confident that the await would chain right to the ?? operator. (I still have trouble reasoning futures sometimes). If it works, ?? Is better than a test because the variable is never in there intermediate null state.

On Sun, Mar 7, 2021, 13:11 Stephen Owens notifications@github.com wrote:

Question for you, in the MethodChannelSpeechToText class you've used this syntax to match null safety requirements:

var result = await _channel.invokeMethod<bool>('has_permission');

return result!;

I don't love the intermediate variable although you're right that appears to be the only clean way to make the syntax work. There's an alternative form, which is this:

return await _channel.invokeMethod<bool>('has_permission') ?? false;

Do you have a preference between the two forms? You may have already thought this through in choosing the syntax you did so let me know.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/csdcorp/speech_to_text/issues/164#issuecomment-792353814, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4FO7DWPFRVQ5RNKMADNLTCPTWHANCNFSM4V7NMRSQ .

[sowens-csd]

I tried a unit test for the null behaviour and I think the ?? version works. I've committed the first test, I'll build out the others now.

[sowens-csd]

I've committed those changes to switch to ?? along with unit tests for them.

[kaladron]

Thanks. Took a longer than that should've, and I'm now setup and working on https://github.com/kaladron/speech_to_text/tree/null-safety

I'll send a PR when I'm closer.

On Mon, Mar 8, 2021 at 7:56 AM Stephen Owens @.***> wrote:

I've committed those changes to switch to ?? along with unit tests for them.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/csdcorp/speech_to_text/issues/164#issuecomment-792851439, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4FO6KGQUY7U35J5KOW53TCTXUDANCNFSM4V7NMRSQ .

[kaladron]

While generated the files, I'm having a problem I don't understand:

Cannot populate the required constructor argument: _alternates. It is assigned to a private field.
package:speech_to_text/speech_recognition_result.dart:64:3
   ╷
64 │   SpeechRecognitionResult(this._alternates, this.finalResult);
   │   ^```

Renaming the field to be public [like so](https://github.com/kaladron/speech_to_text/commit/bd4f6cec9b9b86f1bbfd53d75ba98c6a7bbe9d56) allows it to succeed.

This doesn't seem related to null safety.  Could you please try regenerating the file with the new Flutter stable and see if it's something in my tree?

[Zeswen]

Waiting for this to update our application to Flutter 2.0!

It would be awesome to have it ASAP. Thank you for your great work :)

[kaladron]

Hi Zeswen (and others!), this shouldn't block your adoption of Flutter 2. You need to temporarily disable sound null safety, however. Take a look at my project here:

Add this line:

https://github.com/kaladron/uscis_test/blob/main/lib/main.dart#L15

and then next to your imports:

https://github.com/kaladron/uscis_test/blob/main/lib/learnscreen.dart#L24

And after that it will work. That still allows you to have null safety for the rest of your app while you wait for this one to be updated.

Tks, Jeff Bailey

[sowens-csd]

While generated the files, I'm having a problem I don't understand:

Cannot populate the required constructor argument: _alternates. It is assigned to a private field.
package:speech_to_text/speech_recognition_result.dart:64:3
   ╷
64 │   SpeechRecognitionResult(this._alternates, this.finalResult);
   │   ^```

Renaming the field to be public [like so](https://github.com/kaladron/speech_to_text/commit/bd4f6cec9b9b86f1bbfd53d75ba98c6a7bbe9d56) allows it to succeed.

This doesn't seem related to null safety.  Could you please try regenerating the file with the new Flutter stable and see if it's something in my tree?

Not your tree, I think that's a real error. Let's change it to a public so that you're not blocked. I'll revisit the design of that class in an upcoming release.

[kaladron]

Perfect, thanks.

Tests now compile and run on my branch. A few to fix up: 00:08 +87 -22: Some tests failed.

Looks like mostly the same error, so perhaps I'm close.

On Thu, Mar 11, 2021 at 8:52 AM Stephen Owens @.***> wrote:

While generated the files, I'm having a problem I don't understand:

Cannot populate the required constructor argument: _alternates. It is assigned to a private field.

package:speech_to_text/speech_recognition_result.dart:64:3

╷

64 │ SpeechRecognitionResult(this._alternates, this.finalResult);

│ ^```

Renaming the field to be public like so allows it to succeed.

This doesn't seem related to null safety. Could you please try regenerating the file with the new Flutter stable and see if it's something in my tree?

Not your tree, I think that's a real error. Let's change it to a public so that you're not blocked. I'll revisit the design of that class in an upcoming release.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/csdcorp/speech_to_text/issues/164#issuecomment-796882475, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4FOZXXEX64ZLZ43GDAWDTDDRLNANCNFSM4V7NMRSQ .

[kaladron]

Sent the PR. =) All tests are passing on it.

[kaladron]

It's not really related to this issue, but something I was wondering: Why is the interface library separate from the main one? I couldn't see another library that depended on it.

I was thinking about it because since this is an API break, it seemed like something like a good time to change things.

[sowens-csd]

You mean the speech_to_text_platform_interface? Separating it is good practice according to the Flutter team. It allows other implementers not associated with the main project to depend on it and reimplement the plugin for a new platform.

I'd agree this is a good time to change it as well.

[kaladron]

Is that likely? Or would something just send you patches for, say, Windows support here?

Having them be separate meant that there was a dep that I couldn't easily roll because they both had to go at once. (Can't remember which one off hand, sorry, it's in the commit message and I'm on my phone). Also, that I usually like to do one final test run before I send, but the tied changes meant that I just had to trust that I got it right.

Thanks!

On Fri, Mar 12, 2021, 04:14 Stephen Owens @.***> wrote:

You mean the speech_to_text_platform_interface? Separating it is good practice according to the Flutter team. It allows other implementers not associated with the main project to depend on it and reimplement the plugin for a new platform.

I'd agree this is a good time to change it as well.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/csdcorp/speech_to_text/issues/164#issuecomment-797453545, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4FO2OPWMNYMHDN3B27PDTDHZSTANCNFSM4V7NMRSQ .

[sowens-csd]

I agree that it seems unlikely but it is how the Flutter team has spec'd it. Given that the reality is that no one else is dependent on that, and certainly not on the updated version it's likely safe.

The process, I think, should be to publish the update to the interface lib to pub.dev, then change the dependency to it and test, then if that goes well, publish the primary lib. If you let me know when the interface lib is changed I'll publish it so you can try it out. I found it a bit awkward when I was doing the initial split (as you'll see from the changelog) so I had to keep pushing updates. It's a bit messy out in the open like that, but, it works.

[kaladron]

Eh, I took some time to read the flutter/dart stuff and have been thinking about it all day. I can see their point: Dart is going to support ever more platforms, and that way someone can just add module support to each one following the interface. So folks can rally around one interface and then you don't have to come up with Mac/Windows/Linux/Fuchsia/whatever else comes along.

The hassle comes with things like the PR that I'm about to send you to roll the plugin_platform_interface where each plugin then needs to roll forward, and also whether or not the interface is clear enough or if each platform winds up needing to evolve it.

Ah well, I learned something more about Flutter design today. =)

On Fri, Mar 12, 2021 at 7:34 AM Stephen Owens @.***> wrote:

I agree that it seems unlikely but it is how the Flutter team has spec'd it. Given that the reality is that no one else is dependent on that, and certainly not on the updated version it's likely safe.

The process, I think, should be to publish the update to the interface lib to pub.dev, then change the dependency to it and test, then if that goes well, publish the primary lib. If you let me know when the interface lib is changed I'll publish it so you can try it out. I found it a bit awkward when I was doing the initial split (as you'll see from the changelog) so I had to keep pushing updates. It's a bit messy out in the open like that, but, it works.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/csdcorp/speech_to_text/issues/164#issuecomment-797566141, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4FO2O62HHPHYTOBDVVKLTDIRAHANCNFSM4V7NMRSQ .

[kaladron]

I think everything is set for null safety now. Since this will be a new major version roll, it's a good time for any breaking changes. Do you have anything you want to do? If not, I can send a PR to CP the 1 patch on main that isn't there, and then roll the version (or you can do those if you wish) pub.dev seems to allow uploading a newer version beside things for early access and testing if you want, since you mentioned you had another feature you wanted to get out the door first.

Let me know how I can help!

[sowens-csd]

The changes are looking really good, thanks for all the work. I don't think there's anything important enough to delay these changes. Go ahead with the final PR, then I can run some tests and do a release.

[assylman]

Can't wait for the null safety upgrade of package. Thanks for you job @kaladron!

[sowens-csd]

I've merged your latest PR @kaladron, is it ready for a final test / upload now?

[kaladron]

Please! I ran the example application as part of the update, tested that it transcribed the words. Please do whatever your usual release testing would be and should be good! =)

On Sun, Mar 14, 2021 at 1:47 PM Stephen Owens @.***> wrote:

I've merged your latest PR @kaladron https://github.com/kaladron, is it ready for a final test / upload now?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/csdcorp/speech_to_text/issues/164#issuecomment-798976979, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4FO4M62FUUAWFOALBCPTTDUOFJANCNFSM4V7NMRSQ .

[sowens-csd]

4.0.0-nullsafety is on pub.dev now. Many thanks for taking that on @kaladron.

I'd appreciate if anyone following this could try building your null-safe application against this new version and let me know if you have any problems.

[kaladron]

Working on my app, thank you for being so responsive with the patches!

https://github.com/kaladron/uscis_test/commit/0db372948e43e2701ee6d8960653bddcfc4642ae

This was my last barrier to sound null safety.

On Sun, Mar 14, 2021 at 2:30 PM Stephen Owens @.***> wrote:

4.0.0-nullsafety is on pub.dev now. Many thanks for taking that on @kaladron https://github.com/kaladron.

I'd appreciate if anyone following this could try building your null-safe application against this new version and let me know if you have any problems.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/csdcorp/speech_to_text/issues/164#issuecomment-798983323, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4FOY563TK6STNQR5EHOTTDUTGDANCNFSM4V7NMRSQ .

[sowens-csd]

@kaladron do you want to do the honours of closing this rather lengthy issue?

[kaladron]

Happily, except I don't think I have the rights to. =)

[sowens-csd]

Rats! Okay, I hereby close this issue in @kaladron's name.

[kaladron]

And a good time was had by all. Thanks!