-
Notifications
You must be signed in to change notification settings - Fork 403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generate code that supports implicit-dynamic: false
#557
Comments
implicit-dynamic: false
I've considered this, but it adds a lot of noise to the generated output. Hrm... |
@kevmoo I'm confused by this statement - I'm not sure that the odd extra keywords constitutes considerable noise, and having code that lints successfully under common conditions is surely an improvement. As it is, we're having to hand-modify the generated code for builds to pass, which is obviously very far from optimal. |
My analysis_options.yaml contains strong rules for code writing:
And I do not want to switch off it, bacause of a ton reasons; But currently this plugin is incompatible with this, because generated code isn't compiled. |
Would it be possible to add a property to indicate whether the library generate |
It WOULD be POSSIBLE, but it's a LOT of noise in the generator and in the generated code. It's really hard to do with JSON – which is inherently dynamic. |
We could add an ignore for file at the top of the generated code. This way we won't break analysis for people using it. |
I'm happy to take a PR for that!
…On Wed, Feb 12, 2020 at 3:38 PM Jason Rai ***@***.***> wrote:
We could add an ignore for file at the top of the generated code. This way
we won't break analysis for people using it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#557>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEFCQEEWJ62JICGMFDL6LRCSB7TANCNFSM4JHHN5BA>
.
|
Blocked on dart-lang/source_gen#370 |
See #608 |
I may be misunderstanding, but would a fix for this be to add an explicit Is that the noise you're referring to, @kevmoo? Thanks for helping me understand the stylistic disconnect here. |
It's more than that, @Schoonology . We looked at this already and decided not to do it. You now have an option to ignore these errors in generated code using source_gen See https://github.com/dart-lang/source_gen#configuring-combining_builder-ignore_for_file |
@kevmoo I had the same problem as @trevorwang, so I've used a # build.yaml
targets:
$default:
builders:
source_gen|combining_builder:
options:
ignore_for_file:
- implicit_dynamic_parameter And it works, But, I also use I understand it comes from the fact that both packages use |
Maybe this could be added in the options of the json_serializable builder, something like: targets:
$default:
builders:
json_serializable:
options:
any_map: false
checked: false
# ...
# rules that will be ignored in json_serializable output files
ignored_lint_rules: [] This would allow to use the standard way of configuring the json_serializable builder and to not interfere with other packages that rely on source_gen. |
@mr-mmmmore – I hear you. But then we need to add that option to every generator. We felt it was better to keep it in a common place. ...annoying, I know. |
It seems that the choice you've made imposes to disable the What do you mean with "to keep it in a common place" ? Is there some kind of impossibility to provide this important option? |
Not impossible, just a trade off. Because the ignore is per file, we'd have to plumb logic through every package using source_gen plus source get to enable the setting per generator. We felt it was more straightforward and maintainable to just have the option in source_gen. |
You mean, the |
It's the responsibility of the end user to decide what they want to configure. Doing the work to have |
So, adopting stricter linting rules is my choice and you won't provide support for that? |
We do support it – via configuration of source_gen combining builder. Please keep in mind, @mr-mmmmore – this package is 95% a hobby project for me. I'm very happy to keep it working well for the core scenarios because I know many folks find it useful – including me and folks on my team. At the same time, we don't have a lot of resources to expand features here. We're very focused on the core features of the language, core libraries, and tools that support the Dart and Flutter ecosystems. Thanks for understanading. |
I do. Althought it's frustrating not to be able to follow stricter linter rules because of a single package, the case discussed here is obviously not a major problem, and the main value of the package you've developed is huge, I do not overlook this. My point was that if many packages depend on a single one, there may be more legitimate expectations for this package not to hinder compliancy with the global ecosystem, especially when it comes to implementing the better practices of the ecosystem. As I am new to the Dart/Flutter ecosystem and have to deliver a Flutter app under a time constraint, I can't contribute to this at the moment, but I hope I or someone else will in the future. |
@mr-mmmmore see dart-lang/language#993 I think this is a language issue mostly |
@kevmoo Thank you for implementing this! 🎉 I think it might be good to document this, maybe in the example yaml at the end of the Readme? As I only found out that this was now available by remembering to check back on this issue. |
I couldn't seem to get this to work, in hind-sight I think I needed an IDE restart before the comment would be applied. Anyways, I think I found a better way (or at least simpler), just do this:
No need to use source_gen or make a build.yam, all your generated files can now be loosely typed, while your code can be strict. |
@esDotDev thats great to know, in the past |
Might've been the IDE restart, that bit me for about 2hrs :'( Exclude does not seem to respect any changes until restarting... |
Here's the generated code
And here's the error.
Link: https://circleci.com/gh/trevorwang/retrofit.dart/150?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link
The text was updated successfully, but these errors were encountered: