-
Notifications
You must be signed in to change notification settings - Fork 10
some fixes for strong mode. #47
base: master
Are you sure you want to change the base?
Conversation
Please rebase this on master – eliminate the "merge pull request" |
@@ -81,11 +81,11 @@ class ScriptCompactor { | |||
|
|||
Future<Asset> run() { | |||
var crawler = new ImportCrawler(transform, primaryInput, logger); | |||
return crawler.crawlImports().then((imports) { | |||
return crawler.crawlImports().then((imports) async { |
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.
This async/await stuff shouldn't be necessary afaik, if there is a strong mode error that is probably a bug.
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.
analyzer
wants a (...)- > Asset
and instead it gets a (...) -> Future<Asset>
. They shoud change Future.then
signature to return a dynamic
? Should I send an issue to sdk
or to analyzer
to ignore it ?
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.
Sounds like this is an issue with Future flattening? cc @jmesserly
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.
it's not a flattening bug, it is an inference issue dart-lang/sdk#25944, which I have a CL out, waiting for one more LGTM.
async
is a good workaround, another good one is to pass the type arg explicitly like .then/*<Future<Asset>>*/(...)
Can you also check in the .analysis_options file? |
Actually I'm building it with the |
Would you mind adding an
That helps make sure that we will stay strong mode clean |
should I close this ? |
No description provided.