-
Notifications
You must be signed in to change notification settings - Fork 232
Give error if global activated package depends on hooks #4612
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
base: master
Are you sure you want to change the base?
Conversation
9aa919e
to
30eb18c
Compare
@dcharkes The error message here is quite imprecise and unhelpful. Do we want to give details (which dependency is using hooks?) Do we want to point the user to some documentation? |
Thanks @sigurdm! 🙏
If it's easy, we can name the package that's using hooks.
We should add some info that we're not intending to support hooks in |
@dcharkes If you are happy could you approve? |
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.
LGTM!
() async { | ||
final server = await servePackages(); | ||
server.serve( | ||
'hooks', |
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.
Please use a different name for this, as this https://pub.dev/packages/hooks/versions/0.19.5 should never contain a hook.
}, | ||
); | ||
|
||
test('activating a hosted package gives error if package uses hooks in any' |
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.
Test name needs to be updated. A hook in a dev dep is fine.
Fixes #4609
More refactoring could be used here. But this seems good enough for now.