Fix language overrides

[ThexXTURBOXx]

Continuation of #18 for newer versions of intl_utils.
Most users would benefit from this change since this is a huge bottleneck in using this plugin

Now, this PR also fixes an issue with black frames on rebuild (thanks to @orestesgaolin for providing a fix in his issue already!).
Also, analyzer and lints have been updated to their latest versions, respectively.

Fixes #19
Fixes #90
Fixes #91

---

Sadly, this PR will probably never get merged. See the discussion below for more details.
If you need or want this fix in your own apps, use the following command in order to install it:

flutter pub global activate --source git https://github.com/ThexXTURBOXx/intl_utils.git --overwrite

Please keep in mind that the Flutter Intl plugin for Android Studio does not work with my custom version as it will overwrite my version with the official one. You need to uninstall the plugin (or modify the class files inside it) and generate the language files yourself after installing my version. For that, use the following command:

flutter pub global run intl_utils:generate

[ThexXTURBOXx]

This PR now provides much more additional fixes than before.
Some of them are critical and have a huge impact on usability as can be seen in the referenced issues.
However, I only tested the changes in my apps (where they work perfectly fine).
If there are any issues, please let me know and I will happily be fixing them.

If you want to use this PR in your own apps, use the following command in order to install it:

flutter pub global activate --source git https://github.com/ThexXTURBOXx/intl_utils.git --overwrite

Please keep in mind that the Flutter Intl plugin for Android Studio does not work with my custom version as it will overwrite my version with the official one. You need to uninstall the plugin (or modify the class files inside it) and generate the language files yourself after installing my version. For that, use the following command:

flutter pub global run intl_utils:generate

[fix92]

Thanks for this PR, I also want #91 to be fixed asap. Maybe separate PRs could make this faster instead of providing 1 PR for 3 issues. Thanks!

[ThexXTURBOXx]

@fix92 I also thought that would be the case. However, it seems like PRs are being ignored here lately. Development has also slowed down as it seems. That's why I wanted to fix as many issues as possible to get this package where it is supposed to be :)

[lzoran]

Hi @ThexXTURBOXx,

First of all, thanks for the PR!

I've extracted a fix for outdated dependencies from this PR (released in intl_utils 2.8.0). I hope you don't mind.

Regarding the other two fixes, I'll need to check them in more detail.
I'll get back to you as soon as I check it out.

[ThexXTURBOXx]

@lzoran Thank you for responding! I don't mind at all :)
I will rebase against your new master branch in order to resolve merge conflicts

[ThexXTURBOXx]

I have rebased against master. This PR is now ready to go again!

[ThexXTURBOXx]

I have once again rebased against the main branch. Now this PR is only fixing the language override issue

[lzoran]

Some conclusions after a closer look.

Checked proposed fix for the #90.
It seems that it does not fix the reported problem (the load method still returns Future instead of SynchronousFuture).

Test example

Reproduction steps taken from the similar issue.
Expected: green → blue
Current: green → black → blue

The main.dart file:

import 'package:flutter/material.dart';
import 'package:flutter_localizations/flutter_localizations.dart';
import 'generated/l10n.dart';

void main() => runApp(const AppRoot());

class AppRoot extends StatelessWidget {
  const AppRoot({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return FutureBuilder<bool>(
      // Transition from green -> blue after 3 seconds.
      future: Future.delayed(const Duration(seconds: 3)).then((_) => true),
      builder: (context, snapshot) =>
          // Show a green container first at the first frame.
          !snapshot.hasData ? Container(color: Colors.green) : const MyApp(),
    );
  }
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      localizationsDelegates: const [
        S.delegate,
        GlobalMaterialLocalizations.delegate,
        GlobalWidgetsLocalizations.delegate,
      ],
      supportedLocales: S.delegate.supportedLocales,
      home: Container(color: Colors.blue),
    );
  }
}

Also, this issue should be fixed now with the intl_utils 2.8.1.

Checked proposed fix for the #19.
Setting locale in the Intl.message solves the main issue. However, I'm not sure why we need to remove the usage of the current (reference the translation without context).
Also, the earlier versions of the intl_utils package used the localeName field in the localization class, but I can't remember why we removed it (commit).
Reverting it back again would introduce a breaking change for existing users. The main challenge here is how to migrate them safely in case we decide to apply such change. Also, there are a lot of users who rely on the Flutter Intl plugin for auto-generating localization code, and plugin versioning is not easy (in some cases IDE automatically updates plugins).
Although this is a simple change, it is not easy to adopt.
I think that it is better to wait until we find the right way that best solves this problem and migration than to cause dissatisfaction among the existing users.

[ThexXTURBOXx]

@lzoran Thanks for your answer, however, there are a few things I do not understand entirely:

This PR now does not fix anything related to #90 anymore since it was handled separately as you said.

About the fix for #19: Setting Intl.defaultLocale was removed because otherwise language overrides will instantly set the default locale to a wrong value again (namely the temporary one, overwriting the true default value).
That's why this line needed to get removed.
About the current field being removed: It is possible to add it back in, although I am not sure why this breaking change would be worse or as bad as removing the localeName field in 1.2.1 without mentioning it as a breaking change or marking that release as a new major one.
Additionally, when overriding the language, current will also be changed to the new temporary instance which is not what any user would expect.
Overall, it would be best to get rid of the current entirely or at least deprecate it to indicate that it should not be used anymore because of such problems.
If I add the current field back, there would be no breaking changes in this PR. However, using current could cause problems in the case of language overrides that have also been there before. They are just less likely to occur and will not be permanent anymore.
Should I add it back? Then this PR would no longer break anything, but only bring benefits.

[lzoran]

I think it is best to postpone this change until we find a good enough solution that solves the mentioned problem and the migration of existing users.

[narumi147]

I hope you can think out the "best" solution before flutter/Google died. satire

Just cannot believe such a BUG exists for 2 YEARS without any progress nor positive feedback. The community members have given several solutions that don't breaking anything but no one of your team accept nor commit your ideas. The only thing is we need better solution, we need more thinking...

If you have any question or worry, let's SHARE it, DISCUSS, and SOLVE it. Don't just say NO and keep the issue/pr open for 2 or 10 years. I believe the community members are willing to make the repo the package better.

Sorry for the rude remarks, but that's what I think truly.

[ThexXTURBOXx]

@narumi147 I have also been thinking about this issue for a while now and I truly believe now that it will most likely never be fixed, because my mentioned problems with the current approach won't ever be avoidable when using the (in my opinion) very bad design choice of exposing a static field in a concurrent context.

This is why I have migrated fully to my fork of intl_utils and will keep promoting that one instead.
I also don't understand why an entire class field which was not causing any problems was removed in a minor release like 1.2.1 (the localeName field).
However, we cannot make a major release and remove a static field which causes only problems.
This contradicts itself and makes no sense in my eyes.

[narumi147]

@ThexXTURBOXx Actually, I have no aversion to the static field S.current but do like it, it mainly makes our codes independent from BuildContext and global accessible, this will make codes more clean. Many packages tried to get rid of context from codes, such as store root navigatorKey and use its context.

In most cases, intl messages are used as app UI language, which should be consistent in almost all contexts. So in my folk, I add an optional param {bool override = false}, it will override the static filed current when user change the app language. The Localizations widget is a inherit widget, it will also call S.load, but it won't override current filed since the param default to false. Maybe not so clear, the final result is S.of(context) works fine in Localization inherit widget tree and developer can control S.current when to be overridden.

As for localName or locale field, the same question with yours, I don't understand why it is a breaking change? The S instances are created by generated code rather developer right?

[deakjahn]

@ThexXTURBOXx "Overall, it would be best to get rid of the current entirely or at least deprecate it to indicate that it should not be used anymore because of such problems."

Definitely, emphatically and very strongly: don't you dare to touch current. :-))

There are numerous cases where it's needed, and any solution that removes it makes Flutter Intl completely unusable for many of us. I could probably come up with much more use cases but here are two from my own apps: the first is the very obvious case of background processes without any access to a context at all. The second one is initializing data structures with language dependent values before displaying or independently from any UI, without any access to context again.

Actually, I switched to Flutter Intl from the previously recommended solution mostly because of current and I'm sure not to be the only one.

[ThexXTURBOXx]

@deakjahn While your use case seems valid as an argument, I strongly have to argue that this ties back to not adhering to established software development patterns.
All of the established patterns separate anything that needs translated strings (i.e., frontend/view) from the actual logic (i.e., backend/logic/business/control/etc.).
I would have to think very hard of a single use case where one actually needs to translate strings inside the backend logic where no workaround is possible such as using enums or even strings/other objects as keys and only translating them when wanting to display them in the frontend.

Also, looking around established and well-written Flutter apps, I was not able to find a single one that uses that static field exactly because of adhering to software design patterns and (especially!) because of the problems mentioned above which can easily lead to non-determinism.
Removing this field or (if unwanted) documenting that it is strongly disencouraged to use it would lead to cleaner, better, and (most importantly!) deterministic code overall.

[ThexXTURBOXx]

I have added back the instance, together with a warning that it can lead to non-determinism.
@lzoran This PR is not a breaking change anymore and is ready to get merged.

[deakjahn]

Well, how to put it mildly, without offending anyone. :-)) During decades of development experience, I saw many buzzwords come an go, usually good riddance in the end. So, I couldn't care less about so-called "patterns" and "anti-patterns". :-) You use them as the need comes and you see fit but they're not something you need to use as an argument to base broad decisions on. Please, don't try to impose your views on other developers just because you failed to consider their needs and requirements.

"I would have to think very hard of a single use case where one actually needs to translate strings inside the backend logic where no workaround is possible such as using enums or even strings/other objects as keys and only translating them when wanting to display them in the frontend."

OK, you would have to think very hard. I don't. In the app* I have opened in my IDE right now, I have an Android background service running that needs localized texts and no, it doesn't have any connection to any frontend and UI. Still, it needs them because it has to generate files that contain localized texts. Never shown to the user in a Flutter UI and yet, it exists.

* And, actually, it was halted right now because Flutter Intl doesn't play nice with the brand new SDK, and while coming up with a workaround, I stumbled upon the same problem you mentioned, the AS plugin activating without actually checking whether it's already there, meaning that we can't override it even temporarily...

[ThexXTURBOXx]

I've seen many buzzwords to come along and go away

It is not about buzzwords, it is about things that any developer has at least heard about and any student has learned and used intensively (at least in Germany). Also, I don't think that software design patterns have "gone away". Some of them have been overhauled, but overall they do still exist and are used extensively.

I couldn't care less about so-called "patterns" and "anti-patterns"

As already said, it is not only about "patterns" and "anti-patterns". There are plenty of use cases for singletons, even though they are considered "anti-patterns". Most popular, Minecraft itself is a huge singleton instance.

Please, don't try to impose your views on other developers just because you failed to consider their needs and requirements.

You fail to see why encouraging using something that could lead to non-determinism is a bad thing. I would argue, it is the worst thing any software could suffer from.
This is why either removing it or discouraging its usage are the only good moves I see here.
And yes, I do fail to see any need for this field, which is why I removed it, just to state this open and clearly.

But I do not want to argue about as trivial as this, so I just added it back with a disclaimer that one should re-think his approach when using that instance.

Finally, as said before, I do not see why removing this current field is considered "bad" and a "breaking change" whilst removing the localeName field in 1.2.1 was considered "okay" and "non-breaking" (it was even only a patch version increase!).

Now, let's get this merged and everyone can profit!

[narumi147]

I agree with @deakjahn as I argued before, a static access without context is a popular way. In many cases, you don't have to depend on context, you can find a lot of packages declared that they won't need context in your code, that's because they save the context or instance in a static field.

And second problem, will your solution solve language override via S.current? S.current should be used for app-wide language setting, should not be changed by a Localized widget, but it seems your code will always update S._current for every call of S.load.

As in my solution, S._current can be updated only when null(not initiated) or manually call load+pass overwrite param when user change app language.

But anyway, the owner team didn't fix it for 3 years, even dart 3/flutter 3.10 landed recent days, they are still insusceptible.
The most difficult thing I think is to convince the owner to accept.

[deakjahn]

@narumi147 Anyway, SDK 3 is such a breaking change that either they fix it (my PR is certainly not enough, the plugin has to be modified, too), or there will be enough of us to fork and recreate the IDE plugin. But I don't think that will be necessary, I hope @lzoran will accept the challenge. :-)

I'm in Android Studio with Flutter but mainly because I set it up nicely and I was unwilling to move. I do most of my other stuff in VS Code and have experience with writing plugins there. But let's hope it doesn't come to this.