feat: Add freezed support to feature_brick #12
Conversation
|
Hey @AdamBuchweitz 👋 Thanks for having a look into this! Did you want to go over the changes for this PR? |
|
Oh dang, whoops. I renamed branch to main, not realized that would affect this PR. Sorry about that! |
No problem! Thanks again for opening this. I will take a look later today! |
There was a problem hiding this comment.
First off, thanks again for looking into this @AdamBuchweitz!
(Sorry it took a while to get to this! Crazy week 😁)
It's looking good, but I think ideally the generated (.g) files should also come with the brick so that the developer does not need to run any other generation.
For reference, you can see the model brick and see the .g files generated with it.
I am happy to look into it from here if you need it!
|
Just to make sure I understand what you're asking: you would prefer me to make the brick, then run build_runner to generate the "_state.freezed.dart" and "_event.freezed.dart" files, then inside each of those generated files replace each of the 92 instances of the feature name with {{feature_name.pascalCase()/snakeCase()}}. Is that correct? This is what I have done. This seems like an odd request for a few reasons. Firstly, whenever the Freezed package changes the code it generates, this brick will not match that new output without us maintaining it here as well. Secondly, anyone wanting to use the Freezed option will already be accustomed to generating code. Lastly, I doubt anyone will use this brick as it is without customizing it, in which case they will need to run build_runner anyway. Again, it seems odd, but I don't mind conforming to your repo standards. |
I've had a think about it, and I agree with what you've mentioned. If the developer is wanting to use freezed then generating that code just seems like the better option when talking about freezed bloc (Doing this would not make sense for smaller bricks like model). In this case, I'll have another review in a couple hours and get back to you! Thanks again @AdamBuchweitz! |
| final isBloc = stateManagement == 'bloc'; | ||
| final isCubit = stateManagement == 'cubit'; | ||
| final isProvider = stateManagement == 'provider'; | ||
| final isRiverpod = stateManagement == 'riverpod'; | ||
| final isNone = !isBloc && !isCubit && !isProvider && !isRiverpod; | ||
|
|
||
| bool useFreezed = false; | ||
| if (isBloc) { | ||
| useFreezed = context.logger.confirm( |
There was a problem hiding this comment.
To save developers from more questions, I am thinking of changing this question into a logger.chooseOne to reduce the amount of questions.
So the question would contain the values (Equatable, Freezed, None) - This would only have Freezed as an option if the state management is bloc.
There was a problem hiding this comment.
The wording is a bit tricky on the question, but I think these changes accomplish your idea.
There was a problem hiding this comment.
Thanks! Will take a look later this week!
There was a problem hiding this comment.
Just following up - do you need anything else from me?
There was a problem hiding this comment.
Hey @AdamBuchweitz, Nope! It's all good. I have just been very busy with client work 😅 I will get to this sometime this week!
Brick
Status
READY
Breaking Changes
NO
Description
I used Freezed in conjunction with BLoC, and this commit adjusts the templates to support that. This change is purely additive in that nothing changes whatsoever if you do choose to use Freezed. If you do use Freezed, you don't need Equatable so I dropped that.
The most significant change is that you can no longer
parteverything, since the _event and _state files alreadypartwith their generated code. Therefore you need to import them as you would a standard file.Type of Change