{"action":"created","issue":{"url":"https://api.github.com/repos/dart-lang/pana/issues/58","repository_url":"https://api.github.com/repos/dart-lang/pana","labels_url":"https://api.github.com/repos/dart-lang/pana/issues/58/labels{/name}","comments_url":"https://api.github.com/repos/dart-lang/pana/issues/58/comments","events_url":"https://api.github.com/repos/dart-lang/pana/issues/58/events","html_url":"https://github.com/dart-lang/pana/issues/58","id":247141993,"number":58,"title":"Let's remove LibraryScanner overrides","user":{"login":"isoos","id":4778111,"avatar_url":"https://avatars1.githubusercontent.com/u/4778111?v=4","gravatar_id":"","url":"https://api.github.com/users/isoos","html_url":"https://github.com/isoos","followers_url":"https://api.github.com/users/isoos/followers","following_url":"https://api.github.com/users/isoos/following{/other_user}","gists_url":"https://api.github.com/users/isoos/gists{/gist_id}","starred_url":"https://api.github.com/users/isoos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/isoos/subscriptions","organizations_url":"https://api.github.com/users/isoos/orgs","repos_url":"https://api.github.com/users/isoos/repos","events_url":"https://api.github.com/users/isoos/events{/privacy}","received_events_url":"https://api.github.com/users/isoos/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-08-01T17:22:43Z","updated_at":"2017-08-02T13:10:48Z","closed_at":null,"body":"(1) The override in its current form is misleading, because it creates a `dart:web_safe_io` package which doesn't exists, but somebody may think it does. A `dart-trusted:http` would be a more adept name.\r\n\r\n(2) I think having these overrides is a bad precedent, for the following reasons:\r\n- We don't want to keep track all of the libraries that use `dart:io` in a web-safe way. Getting onto that list, changing it would require efforts on all sides.\r\n- We may track a few, but forget about it. As the libraries, change, some of these information gets outdated, even in a breaking way, and provides misleading results.\r\n- Tracking `package_resolver`'s `http` use is an ever worse precedent: are we going to list everything there that has `http` as its dependency?\r\n\r\nThe correct solution: analyze the Dart sources and detect, whether the client is going to use any of the non-web-related `dart:io` code, and mark it as such. This a lot of effort, and probably not worth it at the moment.\r\n\r\nThe optimistic solution: let's assume that when both `dart:html` and `dart:io` is present, it is importing it in a web-safe-io way. We already have tests for this: https://github.com/dart-lang/pana/blob/master/test/platform_test.dart#L83\r\n\r\nI'd go with optimistic solution, and remove the overrides altogether."},"comment":{"url":"https://api.github.com/repos/dart-lang/pana/issues/comments/319667375","html_url":"https://github.com/dart-lang/pana/issues/58#issuecomment-319667375","issue_url":"https://api.github.com/repos/dart-lang/pana/issues/58","id":319667375,"user":{"login":"isoos","id":4778111,"avatar_url":"https://avatars1.githubusercontent.com/u/4778111?v=4","gravatar_id":"","url":"https://api.github.com/users/isoos","html_url":"https://github.com/isoos","followers_url":"https://api.github.com/users/isoos/followers","following_url":"https://api.github.com/users/isoos/following{/other_user}","gists_url":"https://api.github.com/users/isoos/gists{/gist_id}","starred_url":"https://api.github.com/users/isoos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/isoos/subscriptions","organizations_url":"https://api.github.com/users/isoos/orgs","repos_url":"https://api.github.com/users/isoos/repos","events_url":"https://api.github.com/users/isoos/events{/privacy}","received_events_url":"https://api.github.com/users/isoos/received_events","type":"User","site_admin":false},"created_at":"2017-08-02T13:10:48Z","updated_at":"2017-08-02T13:10:48Z","body":"I'm trying to handle [this test](https://github.com/dart-lang/pana/blob/master/test/end2end/http_data.dart#L13) in a better way. The correct expression is that it may work everywhere, as long as you:\r\n- don't call `new Client()`, and\r\n- don't call any of the top-level `get`, `post`, ... `readBytes` methods, and\r\n- none of the libraries you depend upon do any of these.\r\n\r\nSo, instead of claiming that it works everywhere, maybe we should claim that it may work everywhere. More closely: along the `worksOnWeb` flag, we could also have a `mayWorkOnWeb` flag, and the `description` could be `web?` instead of `web`.\r\n\r\nThat would turn that test assert into `flutter, server, web?`, which I think is more closer to the truth, and it is more generic, than the current hand-picked overrides.\r\n\r\nI'm going to submit a PR with this soon."}}
{"actor":{"display_login":"isoos"}}