Refactor: split BuildAssetUriResolver into AnalysisDriverModel andd AnalysisDriverModelUriResolver.
#3813
Conversation
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. |
…d `AnalysisDriverModelUriResolver`. `AnalysisDriverModel` is `build_runner`'s state related to build steps that use analysis, including an in-memory filesystem for use by the analyzer. `UriResolver` is the analyzer's view of that state.
davidmorgan
force-pushed
the
split-analysis-driver-model
branch
from
8829523 to
974ee94
Compare
|
|
||
| @override | ||
| Uri pathToUri(String path) { | ||
| var pathSegments = p.posix.split(path); |
There was a problem hiding this comment.
I see that this is just copied from the other file but just reading this I have no idea why it should be posix? And why is it strings at all?
I'm not saying change it now (you probably shouldn't with this being splitting stuff out) - just noting it.
There was a problem hiding this comment.
UriResolver is an interface owned by the analyzer that abstracts over a filesystem, it gives it a way to turn URIs into source+paths and paths into URIs.
This class is the view build_runner gives the analyzer of the files it wants to be analyzer.
The paths can be anything as long as they map 1:1 to URIs, so the exact path format is an implementation detail here.
Expanded the class comment to mention some of this:
/// A [UriResolver] on top of [AnalysisDriverModel]'s in-memory filesystem
///
/// This is the analyzer's view of the current build: the files that
/// build_runner wants the analyzer to analyze.
///
/// The in-memory filesystem uses POSIX-style paths with package:foo/bar' /// mapping to /foo/bar`.
| if (pathSegments[2] == 'lib') { | ||
| return Uri( | ||
| scheme: 'package', | ||
| pathSegments: [packageName].followedBy(pathSegments.skip(3)), |
There was a problem hiding this comment.
Again I realize this is just copied from other places, so you don't have to change it here.
I'll just say that we, at least in my opinion, are not doing ourselves any favors by using followedBy and skip. We're creating a list and two iterables.
There was a problem hiding this comment.
Thanks.
The analyzer only calls this O(N) times, where N is the number of files; so that won't matter until we've fixed the O(N^2) behaviour. I look forward to getting to that point ;)
|
|
||
| @override | ||
| Uri pathToUri(String path) { | ||
| var pathSegments = p.posix.split(path); |
There was a problem hiding this comment.
UriResolver is an interface owned by the analyzer that abstracts over a filesystem, it gives it a way to turn URIs into source+paths and paths into URIs.
This class is the view build_runner gives the analyzer of the files it wants to be analyzer.
The paths can be anything as long as they map 1:1 to URIs, so the exact path format is an implementation detail here.
Expanded the class comment to mention some of this:
/// A [UriResolver] on top of [AnalysisDriverModel]'s in-memory filesystem
///
/// This is the analyzer's view of the current build: the files that
/// build_runner wants the analyzer to analyze.
///
/// The in-memory filesystem uses POSIX-style paths with package:foo/bar' /// mapping to /foo/bar`.
| if (pathSegments[2] == 'lib') { | ||
| return Uri( | ||
| scheme: 'package', | ||
| pathSegments: [packageName].followedBy(pathSegments.skip(3)), |
There was a problem hiding this comment.
Thanks.
The analyzer only calls this O(N) times, where N is the number of files; so that won't matter until we've fixed the O(N^2) behaviour. I look forward to getting to that point ;)
For #3811
BuildAssetUriResolveris critical to performance scalability, it's where the import graph is walked and turned into lists of inputs on eachBuildStep.It's very complicated :)
So my plan is to create a new implementation alongside the existing one to work on, and to increase test coverage; I have a "simplest possible implementation" ready in the next PR.
Before then, there is an improvement possible here. This PR splits one thing
BuildAssetUriResolverdoes out, and introduce an interfaceAnalysisDriverModelfor the part I want to re-implement.AnalysisDriverModelisbuild_runner's state related to build steps that use analysis, including an in-memory filesystem for use by the analyzer.UriResolveris the analyzer's view of that state.