[web] Start splitting the engine into smaller libs #25569
Conversation
mdebbar
requested review from
ditman,
ferhatb,
harryterkelsen,
jonahwilliams and
yjbanov
|
I suppose if it builds, you know it works :) How much more do you think you'd be adding onto the SDK rewriter? If it still needs more work, consider adding unit testability for the more complicated find/replace rules |
| import 'engine/navigation/history.dart'; | ||
| export 'engine/navigation/history.dart'; | ||
|
|
||
| import 'engine/navigation/js_url_strategy.dart'; | ||
| export 'engine/navigation/js_url_strategy.dart'; | ||
|
|
||
| import 'engine/navigation/url_strategy.dart'; | ||
| export 'engine/navigation/url_strategy.dart'; |
There was a problem hiding this comment.
Something that might make this files slightly more compact (but still maintaining modularity), is to make a engine/navigation/navigation.dart that exports everything it needs to export from within the navigation "sub-package".
That way, in this more global package, you just need to do: export engine/navigation/navigation.dart.
If you prefer to have more control, you can then show only what's really part of the public API of the engine, vs what navigation was available to engine programmers.
We've done this here, for example:
- https://github.com/flutter/plugins/blob/master/packages/google_maps_flutter/google_maps_flutter_platform_interface/lib/google_maps_flutter_platform_interface.dart (see types/types.dart)
And then, from the google maps, only reexport (via a show) what we explicitly want end users to see from the package above:
(Note that the last link still has two part'ed files, because there's a cross-dependency between them! :P)
There was a problem hiding this comment.
Makes sense! I'll do this.
| import 'package:ui/src/engine/navigation/url_strategy.dart'; | ||
| import 'package:ui/src/engine/services/message_codec.dart'; | ||
| import 'package:ui/src/engine/services/message_codecs.dart'; | ||
| import 'package:ui/src/engine/test_embedding.dart'; |
There was a problem hiding this comment.
Doesn't engine.dart already re-export everything?
There was a problem hiding this comment.
That's like a framework test importing the entire flutter package. Why?
I think grouping them in navigation/navigation.dart and services/services.dart like @ditman suggested makes sense.
There was a problem hiding this comment.
But import 'package:ui/src/engine.dart' show window; is still there, so using show and then importing individual parts seems redundant.
web_sdk/sdk_rewriter.dart
Outdated
| } | ||
| } | ||
|
|
||
| final RegExp _dartLangVersion = RegExp(r'\n// @dart = (.*)'); |
There was a problem hiding this comment.
Can we remove language version specifiers now? /cc @jonahwilliams
There was a problem hiding this comment.
For the package version you can opt in at the SDK constraint level. I'm not sure if we need to do anything else for the SDK version
There was a problem hiding this comment.
@jonahwilliams so I just bump sdk constraint from >=2.12.0-0 <3.0.0 to >=2.13.0-0 <3.0.0, and remove the dart language comments? Or is there another specific sdk version I should use?
| } else { | ||
| // The file doesn't have a dart language version, so we just insert the part | ||
| // directive at the beginning. | ||
| source = 'part of engine;\n' + source; |
There was a problem hiding this comment.
It's strange that some files don't have the language version. Can we make them all consistent?
There was a problem hiding this comment.
All files have the language version. They can't not have it. The build would fail. But I was trying to write the code assuming we may remove language versions at some point.
| import '../ui.dart' as ui; | ||
|
|
||
| part 'engine/alarm_clock.dart'; | ||
| import 'engine/alarm_clock.dart'; |
There was a problem hiding this comment.
We should leave a comment that explains how this file is pre-processed and point to the sdk_rewriter.dart
4 participants
By splitting the engine into smaller libs, we get much faster interactions with the IDE. Things like auto-suggestion and go-to-definition are too slow because the engine is single monolithic library.
This PR includes the tooling changes that make it possible to split the engine. It also migrates several files as a proof-of-concept, and to serve as an example for future migrations.
flutter/flutter#80755