-
Notifications
You must be signed in to change notification settings - Fork 108
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
Add a note about not importable annotations #576
base: master
Are you sure you want to change the base?
Conversation
The build systems run on the Dart VM only, so there are limitations on what imports can be used by builders. Add documentation describing how to work around this limitation by overriding the `TypeChecker` with something other than `fromRuntime`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just provide a GeneratorForMatchingAnnotation
which takes a TypeChecker?
Just as a factory even, would be great. A follow-up is good here. |
The existing design imposes an It would be nice to change that and allow composition but 🤷♂️ Given the current design, I think |
Ya the design I would envision is making a base class that just takes a type checker, and then the existing class would just extend that to make the type checker for you. I wouldn't change anything about how it works generally, just which class you extend would change (and what you pass to the super constructor) |
Adding another class to the public API would increase the surface area of the package and IMO make things harder to find. |
🤷♂️ I don't really agree, I think its significantly better than doing the whole |
I'm not able to find a design I like here with another class (especially because I can't think of a nice name). With a breaking change I could imagine something different, maybe where we drop the generic and you need a constructor when forwards to |
@natebosch – thoughts on this? |
Uh...this is old @natebosch |
@jakemac53 - do you think this is worth landing as is? |
source_gen/CHANGELOG.md
Outdated
@@ -39,6 +42,7 @@ | |||
represents their type. Previously we checked this pattern only for enums, | |||
however there are enum-like usages in classes which are not enums. | |||
- Allow the latest version of `package:analyzer`. | |||
>>>>>>> master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>>>>>>> master |
I still really don't like the |
I don't know if that is always feasible - some annotations could use |
Copy the implementation and test for `GeneratorForAnnotation.`
@jakemac53 - is this implementation of |
The build systems run on the Dart VM only, so there are limitations on
what imports can be used by builders. Add documentation describing how
to work around this limitation by overriding the
TypeChecker
withsomething other than
fromRuntime
.