search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.06k stars 1.56k forks source link

[breaking change] Make `dart:io` `FileSystemEvent` `sealed` in Dart 3.1 #52027

Closed brianquinlan closed 1 year ago

brianquinlan commented 1 year ago

Change Intent

Make FileSystemEvent sealed starting in Dart 3.1.

Justification

It would be nice to be able to exhaustively pattern match on the events produced by FileSystemEntity.watch so FileSystemEvent should be sealed.

Impact

Code targeting Dart 2 will not be able to implements or extends FileSystemEvent (FileSystemEvent is final so Dart 3 code already cannot implement or extend it).

The only known package that implements a FileSystemEvent is package:watcher, which I will fix.

In Dart 3.0.0, the constructors for FileSystemEvent subclasses are public so the only obvious reason for implementing a FileSystemEvent is gone.

Mitigation

Code that implements or extends FileSystemEvent will need to be modified but the modification is straightforward i.e. use the new public constructors for those classes instead of creating your own subclasses.

kevmoo commented 1 year ago

Is this really breaking? I thought sealed was only available in Dart 3?

brianquinlan commented 1 year ago

My reading of the version section of the class modifiers design is that sealed is not ignored even for pre-feature libraries.

kevmoo commented 1 year ago

Oh! This is also a cherry-pick, right?

I lean towards LGTM, but I'd like thoughts from @mit-mit

brianquinlan commented 1 year ago

This isn't a cherry-pick...I'd like the change to be made in Dart 3.1

I was planning on making FileSystemEvent in 3 stages as described in https://github.com/dart-lang/sdk/issues/51912

itsjustkevin commented 1 year ago

@Hixie @grouma and @vsmenon to review this breaking change.

Hixie commented 1 year ago

Fine by me (in particular because this doesn't seem to hurt anyone doing testing).

brianquinlan commented 1 year ago

Ping for @grouma and @vsmenon

vsmenon commented 1 year ago

lgtm

grouma commented 1 year ago

LGTM

elliette commented 1 year ago

@brianquinlan What is the status of the fix for package:watcher mentioned above? webdev serve is currently broken on the main branch of the Dart SDK with the following stack trace (see below).

I think we will need to update the version of package:watcher in build_runner to fix. FYI @jakemac53

CC @annagrin @nshahan 

[INFO] Connecting to the build daemon...
[SEVERE] Failed to build build_runner:build_runner:
[SEVERE] ../../../.pub-cache/hosted/pub.dev/watcher-1.0.2/lib/src/constructable_file_system_event.dart:7:57: Error: The class 'FileSystemEvent' can't be extended, implemented, or mixed in outside of its library because it's a sealed class.
[SEVERE] abstract class _ConstructableFileSystemEvent implements FileSystemEvent {
[SEVERE]                                                         ^
[SEVERE] Failed to build build_runner:build_runner:
[SEVERE] ../../../.pub-cache/hosted/pub.dev/watcher-1.0.2/lib/src/constructable_file_system_event.dart:7:57: Error: The class 'FileSystemEvent' can't be extended, implemented, or mixed in outside of its library because it's a sealed class.
[SEVERE] abstract class _ConstructableFileSystemEvent implements FileSystemEvent {
[SEVERE]                                                         ^
Unhandled exception:
Bad state: Unable to start build daemon.
#0      _handleDaemonStartup.<anonymous closure> (package:build_daemon/client.dart:79:21)
#1      _runUserCode (dart:async/stream_pipe.dart:11:23)
#2      Stream.firstWhere.<anonymous closure> (dart:async/stream.dart:1698:9)
#3      _RootZone.runGuarded (dart:async/zone.dart:1582:10)
#4      _BufferingStreamSubscription._sendDone.sendDone (dart:async/stream_impl.dart:392:13)
#5      _BufferingStreamSubscription._sendDone (dart:async/stream_impl.dart:402:7)
#6      _BufferingStreamSubscription._close (dart:async/stream_impl.dart:291:7)
#7      _SyncBroadcastStreamController._sendDone.<anonymous closure> (dart:async/broadcast_stream_controller.dart:399:22)
#8      _BroadcastStreamController._forEachListener (dart:async/broadcast_stream_controller.dart:322:15)
#9      _SyncBroadcastStreamController._sendDone (dart:async/broadcast_stream_controller.dart:398:7)
#10     _BroadcastStreamController.close (dart:async/broadcast_stream_controller.dart:268:5)
#11     _AsBroadcastStreamController.close (dart:async/broadcast_stream_controller.dart:505:27)
#12     _RootZone.runGuarded (dart:async/zone.dart:1582:10)
#13     _BufferingStreamSubscription._sendDone.sendDone (dart:async/stream_impl.dart:392:13)
#14     _BufferingStreamSubscription._sendDone (dart:async/stream_impl.dart:402:7)
#15     _BufferingStreamSubscription._close (dart:async/stream_impl.dart:291:7)
#16     _SinkTransformerStreamSubscription._close (dart:async/stream_transformers.dart:87:11)
#17     _EventSinkWrapper.close (dart:async/stream_transformers.dart:21:11)
#18     _StringAdapterSink.close (dart:convert/string_conversion.dart:241:11)
#19     _LineSplitterSink.close (dart:convert/line_splitter.dart:141:11)
#20     _SinkTransformerStreamSubscription._handleDone (dart:async/stream_transformers.dart:132:24)
#21     _RootZone.runGuarded (dart:async/zone.dart:1582:10)
#22     _BufferingStreamSubscription._sendDone.sendDone (dart:async/stream_impl.dart:392:13)
#23     _BufferingStreamSubscription._sendDone (dart:async/stream_impl.dart:402:7)
#24     _BufferingStreamSubscription._close (dart:async/stream_impl.dart:291:7)
#25     _SinkTransformerStreamSubscription._close (dart:async/stream_transformers.dart:87:11)
#26     _EventSinkWrapper.close (dart:async/stream_transformers.dart:21:11)
#27     _StringAdapterSink.close (dart:convert/string_conversion.dart:241:11)
#28     _Utf8ConversionSink.close (dart:convert/string_conversion.dart:295:20)
#29     _ConverterStreamEventSink.close (dart:convert/chunked_conversion.dart:78:18)
#30     _SinkTransformerStreamSubscription._handleDone (dart:async/stream_transformers.dart:132:24)
#31     _RootZone.runGuarded (dart:async/zone.dart:1582:10)
#32     _BufferingStreamSubscription._sendDone.sendDone (dart:async/stream_impl.dart:392:13)
#33     _BufferingStreamSubscription._sendDone (dart:async/stream_impl.dart:402:7)
#34     _BufferingStreamSubscription._close (dart:async/stream_impl.dart:291:7)
#35     _SyncStreamControllerDispatch._sendDone (dart:async/stream_controller.dart:792:19)
#36     _StreamController._closeUnchecked (dart:async/stream_controller.dart:647:7)
#37     _StreamController.close (dart:async/stream_controller.dart:640:5)
#38     _Socket._onData (dart:io-patch/socket_patch.dart:2388:21)
#39     _RootZone.runUnaryGuarded (dart:async/zone.dart:1594:10)
#40     _BufferingStreamSubscription._sendData (dart:async/stream_impl.dart:339:11)
#41     _BufferingStreamSubscription._add (dart:async/stream_impl.dart:271:7)
#42     _SyncStreamControllerDispatch._sendData (dart:async/stream_controller.dart:784:19)
#43     _StreamController._add (dart:async/stream_controller.dart:658:7)
#44     _StreamController.add (dart:async/stream_controller.dart:606:5)
#45     new _RawSocket.<anonymous closure> (dart:io-patch/socket_patch.dart:1906:35)
#46     _NativeSocket.issueReadEvent.issue (dart:io-patch/socket_patch.dart:1349:18)
#47     _microtaskLoop (dart:async/schedule_microtask.dart:40:21)
#48     _startMicrotaskLoop (dart:async/schedule_microtask.dart:49:5)
#49     _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:123:13)
#50     _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:190:5)
jakemac53 commented 1 year ago

@elliette can you run a dart pub upgrade? Current constraints in build_runner do allow the latest watcher as far as I can tell.

jakemac53 commented 1 year ago

(this would still be breaking for anybody on an older version of watcher though)

nshahan commented 1 year ago

It appears that there were two breaks that appeared at roughly the same time and we misdiagnosed this change as the cause for the other issue. Jake is right, running a pub upgrade appears to fix the issue with package:watcher and package:build_runner_core has been updated this morning so that problem will be fixed as well. Our tests in the SDK are now passing.