Make pathos run in the browser.

Make pathos run in the browser.

R=rnystrom@google.com

Committed: https://code.google.com/p/dart/source/detail?r=23929

[nweiz, 2013-06-12 00:28:39]

I know working around the lack of configuration-specific libraries using mirrors
squicks you out, but pathos in particular is blocking the stack trace library
and in turn unittest from doing useful path manipulations.

[Bob Nystrom, 2013-06-12 19:55:36]

Kind of strange, but LGTM I guess.

[nweiz, 2013-06-12 20:13:17]

Committed patchset #2 manually as r23929 (presubmit successful).

[ahe, 2013-06-12 21:47:52]

DBC

https://codereview.chromium.org/15967012/diff/2001/pkg/pathos/lib/path.dart
File pkg/pathos/lib/path.dart (right):

https://codereview.chromium.org/15967012/diff/2001/pkg/pathos/lib/path.dart#n...
pkg/pathos/lib/path.dart:43: return
currentMirrorSystem().libraries[Uri.parse('dart:io')];
You shouldn't need to wrap this in a catch block.  It's just a plain map that
should return null if the key isn't there.

https://codereview.chromium.org/15967012/diff/2001/pkg/pathos/lib/path.dart#n...
pkg/pathos/lib/path.dart:56: return
currentMirrorSystem().libraries[Uri.parse('dart:html')];
dart2js will not include the code for 'dart:html' unless explicitly imported. 
But that might be fine for your scenario: presumably the user of pathos will be
using dart:html (otherwise it is a pretty lame web app).

However, you need to be aware of this when you write tests.

[ahe, 2013-06-12 21:52:03]

FWIW, long term, this should work well in dart2js, and the generated code
shouldn't blow up as it does now (assuming you're willing to put a TBD
annotation on the import of dart:mirrors).

But right now, dart2js turns off tree-shaking and global type inference so the
output is huge.  The annotation will tell dart2js that you know what you're
doing and that it shouldn't turn off tree-shaking and type inference.

[nweiz, 2013-06-12 22:20:08]

https://codereview.chromium.org/15967012/diff/2001/pkg/pathos/lib/path.dart
File pkg/pathos/lib/path.dart (right):

https://codereview.chromium.org/15967012/diff/2001/pkg/pathos/lib/path.dart#n...
pkg/pathos/lib/path.dart:43: return
currentMirrorSystem().libraries[Uri.parse('dart:io')];
On 2013/06/12 21:47:52, ahe wrote:
> You shouldn't need to wrap this in a catch block.  It's just a plain map that
> should return null if the key isn't there.

At least when I wrote this, dart2js's MirrorSystem implementation didn't expose
a [libraries] getter at all.

https://codereview.chromium.org/15967012/diff/2001/pkg/pathos/lib/path.dart#n...
pkg/pathos/lib/path.dart:56: return
currentMirrorSystem().libraries[Uri.parse('dart:html')];
On 2013/06/12 21:47:52, ahe wrote:
> dart2js will not include the code for 'dart:html' unless explicitly imported. 
> But that might be fine for your scenario: presumably the user of pathos will
be
> using dart:html (otherwise it is a pretty lame web app).
> 
> However, you need to be aware of this when you write tests.

I'll keep this in mind. Right now this code only works in Dartium; when
dart2js's mirrors are more fleshed-out, I'll add a note to the documentation
mentioning this.

[ahe, 2013-06-12 22:24:57]

https://codereview.chromium.org/15967012/diff/2001/pkg/pathos/lib/path.dart
File pkg/pathos/lib/path.dart (right):

https://codereview.chromium.org/15967012/diff/2001/pkg/pathos/lib/path.dart#n...
pkg/pathos/lib/path.dart:43: return
currentMirrorSystem().libraries[Uri.parse('dart:io')];
On 2013/06/12 22:20:08, nweiz wrote:
> On 2013/06/12 21:47:52, ahe wrote:
> > You shouldn't need to wrap this in a catch block.  It's just a plain map
that
> > should return null if the key isn't there.
> 
> At least when I wrote this, dart2js's MirrorSystem implementation didn't
expose
> a [libraries] getter at all.

Doh!  I'll add that.  I have focused on adding what was needed to get existing
tests running, and it seems like LibraryMirror.libraries is completely untested
:-(