kwado-tech / l10n_mapper

MIT License
4 stars 2 forks source link

Conflicting build_runner changes #22

Closed talamaska closed 6 months ago

talamaska commented 6 months ago

This is not necessarily a bug. More like a request for a way to run this build in a separate build task or be able to set different extension via build.yaml. Like drift was doing.

[SEVERE] Conflicting outputs
Both source_gen:combining_builder and l10n_mapper_generator:l10n_mapper_builder may output lib/app/app.g.dart. Potential outputs must be unique across all builders. See https://github.com/dart-lang/build/blob/master/docs/faq.md#why-do-builders-need-unique-outputs

With drift and mobx I could do something like this , where first drift is run alone, then mobx is run alone

targets:
  $default:
    # disable the default generators, we'll only use the non-shared drift generator here
    auto_apply_builders: false
    builders:
      drift_dev|not_shared:
        enabled: true
        generate_for:
          - "**/db/**.dart"
          - "**/daos/**.dart"
        options:
          data_class_to_companions: false
          use_data_class_name_for_companions: true
          apply_converters_on_variables: true
          generate_values_in_copy_with: true
          new_sql_code_generation: true
      drift_dev|preparing_builder:
        enabled: true
      mobx_codegen|mobx_generator:
        enabled: false
  run_mobx_generator:
    dependencies: ["my_flutter_app"]
    builders:
      drift_dev:
        enabled: false
      drift_dev|preparing_builder:
        enabled: false
      mobx_codegen|mobx_generator:
        enabled: true
        generate_for:
          - "**/mobx/**.dart"

Now I have played a bit with the source code, I'm super noob so I couldn't do much So I first changedbuild_extensions: { ".dart": [".l10n.dart"] } without success From my brief research, it looks like that the LibraryBuilder is not accounting for build_extensions It looks like for you to access the build_extensions data you need access to BuildStep, which is accessible if you write your own builder. It feels as asking for too much from an OS.

And then I changed this config generatedExtension: '.l10n.dart' of the LibraryBuilder, which is used, and and everything is good now. I get the app_localizations.l10n.dart correctly generated. Also it looks like that json file for the mapper is not used anymore. So I configured the build via the build.yaml

l10n_mapper_generator|l10n_mapper_builder:
        options:
          l10n: true
          locale: true
          parseL10n: true
          extension: '.l10n.dart'
          message: "Cannot find translation-key!"
        generate_for:
          include:
            - "lib/l10n/**.dart"

Since build_extensions is not in use anywhere I would pass the extension in the options and expect the the generated file to with it. So I made this change locally

Builder l10nMapperBuilder(BuilderOptions options) {

  print('Received builder options: ${options.config}');

  final l10n = options.config['l10n'] as bool? ?? true;
  final locale = options.config['locale'] as bool? ?? true;
  final parseL10n = options.config['parseL10n'] as bool? ?? true;
  final message = options.config['message'] as String?;
  final String ext = options.config['extension'] as String? ?? '.g.dart';

  return LibraryBuilder(
    L10nMapperGenerator(
      l10n: l10n,
      locale: locale,
      parseL10n: parseL10n,
      message: message,
    ),
    generatedExtension: ext,
  );
}

and added extension: '.g.dart'in the library build.yaml

builders:
  l10n_mapper_builder:
    import: "package:l10n_mapper_generator/l10n_mapper_builder.dart"
    builder_factories: ["l10nMapperBuilder"]
    build_extensions: { ".dart": [".l10n.dart"] }
    auto_apply: dependents
    build_to: source
    defaults:
      options:
        l10n: true
        locale: true
        parseL10n: true
        message: null
        extension: '.g.dart'
kwado-tech commented 6 months ago

Hey @talamaska, I understand this proposal your current take on it is about right with the exception of parsing build-extensions as builder options as this is not feasible.

I'll create a update in a couple of hours to address this and will instead prefer .mapper.dart extension as l10n gives a vague impression of intension.

Will indicate when this has been resolved.

talamaska commented 6 months ago

@kwado-tech I don't have preferences about the suffix. It's just something that came out quickly. Thanks man.

kwado-tech commented 6 months ago

Hey @talamaska, this has been resolved in l10n_mapper_generator: ^2.1.0. Please verify if this resolves it for you and indicate here.

Also give a thumbs-up to the package on pub.dev.

talamaska commented 6 months ago

I'm one of the 2 likes ;) I have some other ideas. I already tested them. For example. I don't need all the translations to be dynamically accessed from the map. Just the form fields validations. So I created a filter and an ability to pass a list of strings with build.yaml. thanks for your work

kwado-tech commented 6 months ago

Great. I'll think about exposing mapper-exceptions to external arguments as this already exists and implicitly defined and consumed.

You can contribute to this as well following the already laid out structure @talamaska

talamaska commented 6 months ago

just to be clear, my idea is the inverse of "exception", maybe a whitelist/blacklist pair is better formulation.