[Feature] support hot reload or programmatically force reload

[narumi147]

Currently, if we change the arb files and re-generate dart files, changes will not take effect using hot-reload, hot-restart does.

But in some develop situations, hot-restart is not desired.

[aleksakrstic]

I agree this would be useful.
However I don't think we will focus on it soon.

If you or anybody else have time for creating a Pull Request for Hot Reload support we would be glad to look at it and merge it.

[narumi147]

Some ideas here.

Translated strings are stored in MessageLookup.messages. It is only created when the library is loaded through such code:

final messages = new MessageLookup();

class MessageLookup extends MessageLookupByLibrary {
  // ...
  final messages = _notInlinedMessages(_notInlinedMessages);

Hot-reload will update the static method _notInlinedMessages, but MessageLookup.messages is only assigned once when library loaded, so we need to update messages manually when hot-reload.

So we can add such a top level function reloadMessages in messages_all.dart, to achieve this, we need to expose a method to call _notInlinedMessages.

Here I add a mixin to MessageLookup

import 'package:intl/message_lookup_by_library.dart';

mixin MessageReloaderMixin on MessageLookupByLibrary {
  void reload();
}

And implement reload in every MessageLookup.

class MessageLookup extends MessageLookupByLibrary with MessageReloaderMixin {
  @override
  void reload() {
    messages.clear();
    messages.addAll(_notInlinedMessages(_notInlinedMessages));
  }
}

Finally, we need to call reload when hot-reload: just call in reassemble in your root stateful widget.

class MyAppState extends State<MyApp>{
  @override
  void reassemble() {
    super.reassemble();
    S.reload();
  }
} 

What's more, why _notInlinedMessages has an unused parameter? why not directly using _notInlinedMessages()

I have tested on my own project, I works. Look into my folk https://github.com/narumishi/intl_utils.

[narumi147]

More, I was using the default config. deferLoading should be disabled, and localizely service is also not used.

@lzoran please take some time to check it.

[lzoran]

Hi @narumishi,

Thanks for the contribution!

I've made a quick check of the proposed solution.
In general, it looks promising. Below I've shared some findings (e.g. potential limitations, issues, etc.) so we can find the best possible solution for this problem.

Findings:

1. The user needs to override reassemble method in one of the "upper" stateful widgets in the tree, to detect hot reload and to trigger reload of the messages. On one hand, that sounds acceptable. But in another, that would pollute code with the logic needed only for development (not sure what are the best practices here, not sure if we can detect hot reload differently).

2. The user needs explicitly to trigger hot reload after generation completes. Differently said, the first hot reload is triggered by the IDE on file save, and the second one by the user when localization code is generated. It would be nice if everything could be done with one hot reload, but I'm not optimistic about that.

3. The proposed solution does not work with the use_deferred_loading set to true. I believe this can be solved by updating the code that generates the reloadMessages function.

4. Regarding integration with the Localizely platform (e.g. the usage of the localizely_sdk package, etc.), I haven't found issues during this quick check. I'll need to perform a more robust check additionally, to be sure to not break some of the existing features.

[narumi147]

@lzoran Thanks for your check.

1. reassemble is only called in development, in release mode, never executed. See flutter SDK doc: https://api.flutter.dev/flutter/widgets/State/reassemble.html
2. Actually I didn't use the IDE plugin, I just ran build script when I need. So I have not check the plugin generation logic, but on your description, it will generate on building and file saved. So if you need such process: save .arb file (ctrl+s will trigger hot-reload in Android Studio)-> generation -> hot-reload again(which is never ).
   On this point, you could look into flutter plugin, how to send hot-reload signal after generation completed. But this should be the feature of IDE plugin rather this dart package?
3. pass
4. pass

[narumi147]

hi @lzoran , any progress here?

[lzoran]

Hi @narumishi,

Sorry for the late reply.

I didn't have much time to invest in research. :(

I really see the benefit of supporting the hot-reload for localization messages.
However, I would like it to be a solution that would not require additional work from the user (of course if something like that is possible).

Also, does the below attached snippet reload localization messages in the same way as the proposed solution?

The main.dart file:

...
import 'generated/intl/messages_en.dart' as messages_en;
import 'generated/intl/messages_de.dart' as messages_de;

...

  // Somewhere in the code...
  @override
  void reassemble() {
    super.reassemble();

    messages_en.messages.messages.clear();
    messages_en.messages.messages.addAll(messages_en.MessageLookup().messages);

    messages_de.messages.messages.clear();
    messages_de.messages.messages.addAll(messages_de.MessageLookup().messages);
  }
  
...

[narumi147]

@lzoran that also makes sense. That's just what I did in MessageReloaderMixin. I was calling _notInlinedMessages directly, and your are creating a new instance instead. Yours is more concise.

In my example, the above reloading is done in message_all. reloadMessages, and S.reload is a shortcut for that.

However, I would like it to be a solution that would not require additional work from the user (of course if something like that is possible).

Since your library.messages is a Immutable final, you must have to call reload in reassemble. In my example, user only needs to add the following code, I don't think it's complex anyway.

class MyAppState extends State<MyApp>{
  @override
  void reassemble() {
    super.reassemble();
    S.reload();
  }
} 

If you want nothing to be added, you need to make messages as a getter which is higher cost.

Detect changes and auto-reload when getting S.delegate

The above method(reload in reassemble) will reload every hot-reload.
This following method will only reload when changed.

Why I detect changes in S.delegate getter is, S.delegate must be passed to MaterialApp during its build, and hot-reload will also trigger re-build.

class S {
  static String get _hash => 'abcdabcd';  // generated hash or timestamp
  static String? _lastHash;

  static AppLocalizationDelegate get delegate {
    if (_lastHash != null && _lastHash != _hash) {
      reloadMessages();
    }
    _lastHash = _hash;
    return AppLocalizationDelegate();
  }
}

0. Generate a hash value or timestamp for each code generating.
1. S.delegate must be called when passing it to MaterialApp for each building(also hot-reload)
2. if _lastHash is null, it means it's the first building, so you don't need to reload messages.
3. If _hash is changed, then you need to reload messages.

Future implements

1. deferred_loading, I hope you can implement this part, I know nothing about this yet.

[narumi147]

@lzoran I have changed something. Test passed on my project, note that, I didn't use vscode/jetbrain plugin, just build in terminal and hot-reload.

Conclusion

This PR(narumishi/intl_utils) won't need any additional code for user - just what you wanted.

Changes

1. directly update messages in message_all.reloadMessages
2. support deferred loading, still have an issue to discuss, see next part point 1
3. auto-reload when getting S.delegate only in debug mode - inside assert
4. auto-reload only called when intl_utils rebuilt - using timestamp hash as a tag for each build
5. make _deferredLibraries as a getter not library variable - fix the bug when switch setting use_deferred_loading from true to false, causing library loader not defined

Changed generated code:

// messages_all.dart
Map<String, LibraryLoader> get _deferredLibraries => {
      'en': () => new Future.value(null),
      'ja': () => new Future.value(null),
      'zh': () => new Future.value(null),
    };

Future<void> reloadMessages() async {
  for (final lib in _deferredLibraries.values) {
    await lib();
  }
  messages_en.messages.messages.clear();
  messages_en.messages.messages.addAll(messages_en.MessageLookup().messages);
  messages_ja.messages.messages.clear();
  messages_ja.messages.messages.addAll(messages_ja.MessageLookup().messages);
  messages_zh.messages.messages.clear();
  messages_zh.messages.messages.addAll(messages_zh.MessageLookup().messages);
}

// l10n.dart
class S {
  // other codes

  static Future<void> reload() async {
    reloadMessages();
  }

  static String get _hash => '971794012';
  static String? _lastHash;

  static AppLocalizationDelegate get delegate {
    assert(() {
      if (_lastHash != null && _lastHash != _hash) {
        reloadMessages();
      }
      _lastHash = _hash;
      return true;
    }());
    return const AppLocalizationDelegate();
  }
  // other codes
}

Future stuffs

Known issues for future development, but maybe not related to this pr.

1. hot-reload procedure (when use_deferred_loading=true): call MaterialApp widget build -> call reloadMessages(deferred) -> widget build finish -> reloadMessages finish -> i18n not updated in this building frame. This only happened when some lib never loaded before reloadMessages. Anyway, hot-reload again will show the correct result - it's not a so important thing.
2. to reload messages, I have to loaded all deferred libs even only lib loaded before.
3. S.delegate is not const, it's a getter now. I think it doesn't matter? And maybe it's not reasonable for putting reloading code here. But this is the only code when config a MaterialApp to support l10n.
4. For plugin only, plugin will watch file changes and rebuild code, after new code built, it should trigger hot-reload. This is better experience, but it's a plugin's issue rather this lib's issue.

If you agree with the solution, I could create a pr. Anyway, I don't think there is something block this feature.

[lzoran]

Thanks for such detailed research and PR @narumishi!

I think we should not rush with changes in generated code (especially if the same can be done without modification).
Once something is added, it is difficult to change it later.

As I said above, I really see the benefit of such feature.
I suggest leaving this open until we are sure that this is the best way to achieve hot-reloading of localization messages.
In the meantime, those who really need this can use your fork or the workaround shown above.

What do you think @aleksakrstic?

[aleksakrstic]

Yes, lets leave this open for now.