-
Notifications
You must be signed in to change notification settings - Fork 276
Fix: Migrate off legacy JS/HTML APIs #750
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
Conversation
PR Health
Breaking changes
|
Package | Change | Current Version | New Version | Needed Version | Looking good? |
---|---|---|---|---|---|
grpc | Breaking | 4.0.1 | 4.0.2-wip | 5.0.0 Got "4.0.2-wip" expected >= "5.0.0" (breaking changes) |
This check can be disabled by tagging the PR with skip-breaking-check
.
Changelog Entry ✔️
Package | Changed Files |
---|
Changes to files need to be accounted for in their respective changelogs.
Coverage ⚠️
File | Coverage |
---|---|
lib/grpc_or_grpcweb.dart | 💚 57 % |
lib/src/client/transport/xhr_transport.dart | 💔 Not covered |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check
.
API leaks ⚠️
The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
Package | Leaked API symbols |
---|---|
grpc | $1.Duration Any |
This check can be disabled by tagging the PR with skip-leaking-check
.
RFC. There's quite a lot of boilerplate to work around the Help needed: the tests don't pass. I'm not sure how they're supposed to be run, since I run |
@Moseum - anyone on maintainer side who could help shepherd this? |
I think we need to land #747 first, which has some failures at the moment. Happy to see this go through in general, thanks a lot for the initiative! |
…nsport to support testability
05eea0d
to
c7b9125
Compare
I see different failures locally - |
a181372
to
5fee651
Compare
Happy new year! Please let me know if there's anything else I can do to assist here. |
Gentle reminder this is here. Anyone who wants to use grpc + wasm mode needs this. |
Aran, did you get the tests passing and your RFC addressed from your #750 (comment) above? I really appreciate all the work on this. Looking forward to being able to upgrade to WASM! |
Sorry for the long wait @aran ... I tried to refactor the test to use the mocking as described in https://dart.dev/interop/js-interop/mock, but the lack of analyzer support for overrides and other small problems makes this not a nice solution IMO. So this LGTM! Please do adapt the PR title and description, and rebase the PR. Then we can merge it ASAP |
5fee651
to
b39a6ff
Compare
@mosuem updated. Let me know if there's anything else. |
Thanks for seeing this through! |
Thank you! |
Fixes #715.
This includes a rebase of @minoic's #730 onto the latest master, along with a couple updates.
It also updates client_xhr_transport_test to avoid dart:html, performing some ceremony to work around the unavailability of the previous mock strategy.