Skip to content

Conversation

@vasilich6107
Copy link

Fixes test run by locking the build version
adds annotations handling for deprecations pass through

@vasilich6107 vasilich6107 force-pushed the deprecated-handling branch from fe078bf to 970dc47 Compare June 24, 2025 18:15
@vasilich6107 vasilich6107 force-pushed the deprecated-handling branch from 970dc47 to 092e156 Compare June 24, 2025 20:39
@Rongix
Copy link
Collaborator

Rongix commented Jun 26, 2025

Hi, thanks for the suggestion.

Could you clarify the use case for this feature? If the purpose is to mark properties as deprecated, this is not the best approach, as in Dart, deprecation must be applied directly in the class where the property is defined. Applying 'Deprecated' in the mixin is not sufficient, as it does not propagate correctly to where it matters for consumers.

Let me know the full context of what you’re trying to solve so we can evaluate it properly.

@vasilich6107
Copy link
Author

vasilich6107 commented Jun 26, 2025

Hi @Rongix
When we deprecate prop in the class it does not propagate properly to copyWith
So anyone who uses copyWith for altering the theme will not be aware about deprecation

image

@vasilich6107
Copy link
Author

Hi @Rongix
Could you check my clarification

@vasilich6107
Copy link
Author

@Rongix
Could you give a feedback

Copy link
Collaborator

@Rongix Rongix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi. I have 2 requests before we can merge it.

  1. Make sure you don't format old code, that will be formatted later on before releasing to pub via separate commit.
  2. Changes to how to handle deprecation:
  • Try to extract deprecation message and store it in the field (similar way it is done with documentation)
  • When generating copyWith method, construct @Deprecated a new from deprecation message
  • Add an extension that handles deprecation message instead of this
            .where((m) => m.toSource().startsWith('@Deprecated'))
            .map((m) => m.toSource())
            .toList(growable: false),

name: e.displayName,
type: e.type.getDisplayString(withNullability: true),
documentation: e.documentationComment,
annotations: e.metadata
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could add a field deprecation instead, in a same manner as documentation is added. It is a language feature and I don't think we need to add handling for other annotations. Also it would be better if we were able to check @deprecated type instead checking if it string starts with @Deprecated

  • see how isThemeExtensionType is implemented, we would benefit from method that is also extension on type but gets deprecation message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants