Skip to content

Remove dart:mirror usage from package:http #1

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
mkustermann opened this issue Jan 7, 2015 · 21 comments
Closed

Remove dart:mirror usage from package:http #1

mkustermann opened this issue Jan 7, 2015 · 21 comments
Labels
status-blocked Blocked from making progress by another (referenced) issue

Comments

@mkustermann
Copy link
Member

A change to the dart2js compiler introduced a warning on mirror usage in dart code -- see Emit warning on import of dart:mirrors.. According to the warning, users will be forced to pass an additional flag to the dart2js compiler when their code is using dart:mirrors. This affects all users of package:http.

When compiling the following example

import 'package:http/http.dart';
main()=> print("hello");

dart2js will print out the following warning:

Warning: 
****************************************************************
* WARNING: dart:mirrors support in dart2js is experimental,
*          and not recommended.
*          This implementation of mirrors is incomplete,
*          and often greatly increases the size of the generated
*          JavaScript code.
*
* Your app imports dart:mirrors via:
*   x.dart => package:http => dart:mirrors
*
* Starting with Dart 1.9, you must use the
* --enable-experimental-mirrors command-line flag to opt-in.
* You can begin using this flag now if mirrors support is critical.
*
* To learn what to do next, please visit:
*    http://dartlang.org/dart2js-reflection
****************************************************************

That flag does not seem to be enforced as of SDK version 1.9.0-dev.1.0+google3-trunk, but that seems to be only a matter of time.

@nex3
Copy link
Member

nex3 commented Feb 4, 2015

This is blocked on having a viable alternative (issue 6943). I believe the end result of the discussion about this warning was that it also shouldn't be introduced until a viable alternative to mirrors exists.

@nex3 nex3 added the status-blocked Blocked from making progress by another (referenced) issue label Feb 4, 2015
@skybrian
Copy link

I'm told this warning is spurious and dart2js should be fixed to only print it when code size is actually affected.

@skybrian
Copy link

Here's the bug for fixing dart2js:
https://code.google.com/p/dart/issues/detail?id=23293

@anders-sandholm
Copy link

Just wondering, how can we get package http to not use mirrors, such that it can be used "out of the box" by Flutter?

See also this discussion flutter/flutter#5831 (comment)

Given that cross-platform package support is not there, what is the best viable way to solve this in the short term?

@anders-sandholm
Copy link

Having talked to @floitschG it seems we might be able to rewrite things in a way that uses config-specific imports instead of mirrors.

Config-specific imports are tracked here dart-lang/sdk#24581.
DDC is currently still missing (dart-archive/dev_compiler#595).

@matanlurey
Copy link
Contributor

Nice!

@nex3
Copy link
Member

nex3 commented Sep 13, 2016

I'm definitely planning on rewriting this with config-specific imports—I
even have a local branch that suits it. But the the feature needs to no
longer be behind a flag, and to be supported by DDC, for that branch to be
releasable.

On Sep 13, 2016 4:47 PM, "Matan Lurey" [email protected] wrote:

Nice!


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAAvAlQUSEtCGWwV4_53otKYBeub7dNks5qpsWcgaJpZM4DPVeo
.

@chinmaygarde
Copy link
Member

Flutter would be extremely excited to finally use the package directly. If you are willing to publish the local branch separately, we can even test it out for you by specifying the package as a Git dependency.

Thanks.

@sethladd
Copy link
Contributor

Thanks @nex3 ! We'd love to DEP in your branch so we can use http again.

@eseidelGoogle
Copy link

Flutter has checked in a forked copy of http/http.dart for the time being:
flutter/flutter@400585c

We'll happily remove it as soon as this is resolved. Thanks!

@nex3
Copy link
Member

nex3 commented Sep 28, 2016

I've pushed my local changes in the cross-platform branch. Heads up: it's not especially thoroughly tested.

@sethladd
Copy link
Contributor

Thanks!

On Tue, Sep 27, 2016 at 5:32 PM, Natalie Weizenbaum <
[email protected]> wrote:

I've pushed my local changes in the cross-platform branch
https://github.com/dart-lang/http/tree/cross-platform. Heads up: it's
not especially thoroughly tested.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAVZ3VldY86sxSj0OmF0dUuwkzmWV-qks5qubWrgaJpZM4DPVeo
.

@eseidelGoogle
Copy link

Curious for an update? Folks are attempting to use package:http-dependent packages in Flutter and are I believe blocked by this bug.

Theoretically users could use dependency_override to override package:http to point to packages/flutter/lib/src/http, but I've not actually seen that work yet.
https://www.dartlang.org/tools/pub/dependencies#dependency-overrides

@sethladd
Copy link
Contributor

An internal dev got dependency overrides working, but we needed to create a fake http package which imports Flutter's http package and reexports everything. Then, we put that fake http package onto git, so it could be pulled down during pub's download/resolution step.

@nex3
Copy link
Member

nex3 commented Nov 18, 2016

This is still blocked on getting interface libraries implemented everywhere and out from behind the flag.

@eseidelGoogle
Copy link

@nex3 could you remind me which issues(s) those are? Presumably some subset of the ones mentioned in earlier comments?

@nex3
Copy link
Member

nex3 commented Nov 18, 2016

dart-lang/sdk#24581 is the tracking issue for config-specific imports.

@mit-mit
Copy link
Member

mit-mit commented Dec 16, 2016

@nex3 removing blocked field, as per dart-lang/sdk#24581 (comment) all main pieces have landed and verified in a new test case.

Can we try out this package change now?

@mit-mit mit-mit removed the status-blocked Blocked from making progress by another (referenced) issue label Dec 16, 2016
@nex3
Copy link
Member

nex3 commented Dec 19, 2016

@mit-mit There's not much we can do until the feature is available in a stable release.

@mit-mit
Copy link
Member

mit-mit commented Dec 20, 2016

Oh, good point. That is scheduled for late January!

@mit-mit mit-mit added the status-blocked Blocked from making progress by another (referenced) issue label Dec 21, 2016
natebosch pushed a commit that referenced this issue Apr 13, 2021
brianquinlan referenced this issue in brianquinlan/http May 24, 2022
brianquinlan referenced this issue in brianquinlan/http Jun 1, 2022
brianquinlan referenced this issue in brianquinlan/http Jun 2, 2022
brianquinlan referenced this issue in brianquinlan/http Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status-blocked Blocked from making progress by another (referenced) issue
Projects
None yet
Development

No branches or pull requests

9 participants