Skip to content

Update the use of 'package:shelf_web_socket's webSocketHandler method#2421

Merged
devoncarew merged 3 commits into
masterfrom
widen_dep_shelf_web_socket
Dec 3, 2024
Merged

Update the use of 'package:shelf_web_socket's webSocketHandler method#2421
devoncarew merged 3 commits into
masterfrom
widen_dep_shelf_web_socket

Conversation

@devoncarew

Copy link
Copy Markdown
Contributor
  • add a 2nd argument to the closure passed into package:shelf_web_socket's webSocketHandler method
  • widen the dep on package:shelf_web_socket

This will allow us to add more type info to the closure that webSocketHandler expects; it's currently an untyped Function. See also dart-lang/shelf#457 and dart-lang/shelf#463.

This forward declares compatibility with 3.0 of package:shelf_web_socket; I think this is necessary - as dart test uses both package:test and package:shelf_web_socket - but happy to hear otherwise.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@devoncarew devoncarew requested a review from a team as a code owner December 3, 2024 18:42
@github-actions

github-actions Bot commented Dec 3, 2024

Copy link
Copy Markdown

PR Health

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

@jakemac53

jakemac53 commented Dec 3, 2024

Copy link
Copy Markdown
Contributor

We should be able to land this change without widening the constraint pre-emptively, and then in shelf_web_socket we can add a dependency override on the latest version of package:test (with the forwards compatible change).

Then, when shelf_web_socket is published, we can update the constraint here and publish again (after which point the override in shelf_web_socket can be removed).

That avoids publishing any regular dependency constraints in any packages with versions that aren't actually there yet (it will only be an override on the dev dependency in shelf_web_socket, which is fine).

WDYT?

@devoncarew

Copy link
Copy Markdown
Contributor Author

That avoids publishing any regular dependency constraints in any packages with versions that aren't actually there yet (it will only be an override on the dev dependency in shelf_web_socket, which is fine).

Sounds great! Forward declaring the dep - even for something we control - does feel like crossing your fingers and hoping.

Comment thread pkgs/test/pubspec.yaml Outdated
@devoncarew devoncarew changed the title widen the dep in package:shelf_web_socket Update the use of 'package:shelf_web_socket's webSocketHandler method Dec 3, 2024
Comment thread pkgs/test/lib/src/runner/browser/compilers/dart2js.dart
Comment thread pkgs/test/CHANGELOG.md Outdated
devoncarew and others added 2 commits December 3, 2024 11:09
@devoncarew

Copy link
Copy Markdown
Contributor Author

Is it expected here that the publish check fails? We just ignore for this package, but still use publishing automation after this lands?

 Package validation found the following potential issues:
  * Your dependency on "test_api" should allow more than one version. For example:
...

@jakemac53

Copy link
Copy Markdown
Contributor

Is it expected here that the publish check fails? We just ignore for this package, but still use publishing automation after this lands?

Yes, we have to add the label to ignore warnings because we pin test_core and test_api.

@github-actions

github-actions Bot commented Dec 3, 2024

Copy link
Copy Markdown

Package publishing

Package Version Status Publish tag (post-merge)
package:test 1.25.11 ready to publish test-v1.25.11
package:checks 0.3.1-wip WIP (no publish necessary)
package:fake_async 1.3.2 already published at pub.dev
package:matcher 0.12.18-wip WIP (no publish necessary)
package:test_api 0.7.4 already published at pub.dev
package:test_core 0.6.7 already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

@devoncarew devoncarew merged commit 2096773 into master Dec 3, 2024
@devoncarew devoncarew deleted the widen_dep_shelf_web_socket branch December 3, 2024 19:27
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Dec 3, 2024
lints (https://github.com/dart-lang/lints/compare/2e1321e..e1d4794):
  e1d4794  2024-12-03  Michael Goderbauer  Revert "Update README.md before archiving (`#214`)" (dart-archive/lints#219)

shelf (https://github.com/dart-lang/shelf/compare/657ebd3..2b5b683):
  2b5b683  2024-12-02  Devon Carew  move analysis_options.yaml into packages (dart-lang/shelf#460)

sse (https://github.com/dart-lang/sse/compare/befbd6d..b97dc3a):
  b97dc3a  2024-12-02  dependabot[bot]  Bump dart-lang/setup-dart in the github-actions group (dart-archive/sse#119)

test (https://github.com/dart-lang/test/compare/c2a6986..2096773):
  20967732  2024-12-03  Devon Carew  Update the use of 'package:shelf_web_socket's `webSocketHandler` method (dart-lang/test#2421)
  1d28d738  2024-12-02  Ben Konyi  Support `package:vm_service` 15.x (dart-lang/test#2420)

Change-Id: I852515f46f12b53d2a2fe665a57f7ddcfde36222
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398605
Commit-Queue: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants