Skip to content

polymer: turn on MirrorsUsed for all polymer libs #12396

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

Closed
jmesserly opened this issue Aug 12, 2013 · 19 comments
Closed

polymer: turn on MirrorsUsed for all polymer libs #12396

jmesserly opened this issue Aug 12, 2013 · 19 comments
Assignees
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures
Milestone

Comments

@jmesserly
Copy link

this package could benefit greatly, since it only reflects on things that are Observable

@sethladd
Copy link
Contributor

Removed Priority-Unassigned label.
Added Priority-High label.

@sethladd
Copy link
Contributor

Issue #12730 has been merged into this issue.

@sethladd
Copy link
Contributor

I've attached a very simple app with a single custom element. I don't think I use observing at all. After running pub deploy, the output JS file is 1.8MB.


Attachment:
deleteme2.zip (817.48 KB)

@larsbak
Copy link

larsbak commented Aug 27, 2013

Added this to the M7 milestone.

@sethladd
Copy link
Contributor

Issue #12917 has been merged into this issue.

@sigmundch
Copy link
Member

Marked this as blocking #12917.

@jmesserly
Copy link
Author

so, ideally we'd add a few lines:

// in package:observe
@MirrorsUsed(targets: 'observe.Observable', override: 'observe')

// in package:polymer
@MirrorsUsed(targets: 'polymer.Polymer', override: 'polymer')

// in package:polymer_expressions
@MirrorsUsed(targets: 'observe.Observable', override: 'polymer_expressions')

unfortunately this doesn't work currently because mirrorsused doesn't select subtypes

@jmesserly
Copy link
Author

Marked this as being blocked by #13283.

@jmesserly
Copy link
Author

edit subject; this is a tracking bug for all Polymer.dart code that uses reflection.

We should be able to compile a Polymer app in dart2js with no hints about mirrors, and code size should be small.


cc @justinfagnani.
cc @sigmundch.
Removed Library-Observe label.
Changed the title to: "polymer: turn on MirrorsUsed for all polymer libs".

@kasperl
Copy link

kasperl commented Oct 4, 2013

We're making progress on implementing support for meta targets in MirrorsUsed. Try patching in https://codereview.chromium.org/26007002/ and use something like:

   @­MirrorsUsed(metaTargets: 'Observable')
   import 'dart:mirrors';

@sigmundch
Copy link
Member

Removed this from the M7 milestone.
Added this to the M8 milestone.

@sigmundch
Copy link
Member

Issue #12917 has been merged into this issue.

@sigmundch
Copy link
Member

Unmarked this as blocking #12917.

@sigmundch
Copy link
Member

Added Library-PolymerBuild label.

@jmesserly
Copy link
Author

@jmesserly
Copy link
Author

(this affects observe/polymer/polymer_expressions)


Removed Library-PolymerBuild label.
Added Library-Polymer, Library-Observe, Library-PolymerExpressions labels.

@jmesserly
Copy link
Author

Set owner to @jmesserly.
Added Started label.

@jmesserly
Copy link
Author

Added Fixed label.

@jmesserly jmesserly added Type-Defect P1 A high priority bug; for example, a single project is unusable or has many test failures labels Oct 15, 2013
@jmesserly jmesserly self-assigned this Oct 15, 2013
@jmesserly jmesserly added this to the M8 milestone Oct 15, 2013
@DartBot
Copy link

DartBot commented Jun 5, 2015

This issue has been moved to dart-archive/observe#31.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

6 participants