search

dart-gde / chrome.dart

Provide Dart interop with chrome.* APIs for Chrome Packaged Apps
Other
125 stars 48 forks source link

Fix to use Dart 2 core library lowercase constant names. #272

Open munificent opened 6 years ago

adambender commented 6 years ago

Hey Bob it looks like this change is failing the build. It looks like it is just a couple unused imports, would you mind fixing that?

munificent commented 6 years ago

Those unused import hints are cascaded errors because it doesn't realize that jsonDecode() is defined in there. That's because Travis is trying to use an older version of Dart 1.x stable.

I could change the config to use the latest dev channel build but that's going to cause massive failures. The code generation tools in this package are nowhere near being strong mode clean or Dart 2 ready. I spent some time trying to make it strong mode clean but it was a rathole and I gave up. The problem is that it's using Parser in a very dynamic way (basically using Parser<dynamic> everywhere instead of a typed parser) and passing in callbacks that expect typed parameters. That doesn't fly in Dart 2. To fix that, you need to either type all the parsers correctly or loosen all of those callbacks to take dynamic. The latter is gross — it makes the package even less typed than it is now. I tried the former, but it looks like there are a couple of places in the grammar where sometimes it parses a single value and sometimes a list of values. I didn't have enough context to figure out how to type those.

The generated code that is used at runtime is fine in Dart, though, which is what really matters for users of the package.

adambender commented 6 years ago

Appreciate that you took even that much of a look. In truth, none of the current maintainers (none of us were original authors) have enough context to be able to address most of the Dart 2 related issues because as you point out, this package really takes advantage of the dynamism of Dart 1.

Let me circle back with Ben and Oscar and see what we think can be done. Obviously, getting constant fixed should be an easy thing to do, but this PR is bringing a latent issue to the forefront and I wanna make sure we have a plan.

munificent commented 6 years ago

For what it's worth, I got really close to having the whole thing Dart 2 clean. There were only a couple of grammar productions that were weird. So it should be doable and it's mostly a mechanical effort — adding lots of type arguments to places that currently just say Parser.

kevmoo commented 6 years ago

@munificent – just need to switch travis over to use dev instead of stable...or wait until next week 😄

tonyclickspace commented 6 years ago

Dart 2 landed, this should pass checks now?