We should remove the dependency on package:_fe_analyzer_shared

[devoncarew]

package:_fe_analyzer_shared is an internal implementation detail of the analyzer package (and the CFE). It's not versioned using semver, and, depending on it can cause version lock issues for the ecosystem. That happened recently with Flutter, when they tried to use the latest analyzer (to capture some work on super parameters; see #1573 and flutter/flutter#100575 (comment) for context).

The dep I see is here: https://github.com/dart-lang/webdev/blame/079a52ec34ffb13b1239779ab6b57d13c2ae635a/dwds/lib/src/utilities/dart_uri.dart#L10 (added ~4 months ago). @annagrin @helin24 @elliette - you all may have context here. What do you think is the best way to remove this dep? I'm not sure what functionality we're trying to get with it. Is this something that the analyzer itself could implement? Or, that it could export from the _fe_analyzer_shared package?

[jacob314]

Fyi @grouma who probably knows why it was added.

[stereotype441]

(I made this comment on internal analyzer team chat; pasting it here for wider visibility)

Yeah, +1 to finding a way to prune that dep. We pretty much consider all of package:_fe_analyzer_shared to be a private API with zero compatibility guarantees (which is why evereything is under lib/src and we always bump the major version when we publish it). Which means that any package other than analyzer that depends on it is going to have a bad time

(Analyzer only gets away with it because we publish analyzer and _fe_analyzer_shared in lock step with one another)

[natebosch]

I was pointed to #1290 as the planned solution when I brought this up before.

[annagrin]

We are using package:_fe_analyzer_shared to resolve dart: uris using libraries spec. I believe IntelliJ and VSCode are using the API implemented in #1458.

We could either not resolve them (not sure what the consequences would be) or maybe provide this functionality in some other way. Options to investigate:

• Not resolve dart: URIs. @helin24 would it break IntelliJ? @DanTup would it break dart code?
• Kernel might have the information - @johnniwinther am I correct? (If yes, we would need to add an API to the expression compiler and implement it in the frontend server and ddc)
• Try find the information from the source maps, if any of them are loaded at the point we need them (not sure here, need to investigate). Also, would this option impact performance?
• @stereotype441 do you think it is possible to provide reading libraries spec separately from the analyzer package?
• Store more metadata (including SDK) - see Create app-level metadata for communication between the build system and debugger #1290. This would not be addressed soon though, so we might first consider options above first since removing dependency on package:_fe_analyzer_shared seems to be more urgent.

[devoncarew]

I don't know how stable that API is (package:_fe_analyzer_shared/src/util/libraries_specification.dart) or how stable that libraries file format is (it just looks like a json encoding). But modulo some of the ideas above (which could be strictly better), two other things to consider are:

• to have package:analyzer export the LibrariesSpecification class from the package:_fe_analyzer_shared/src/util/libraries_specification.dart library. Then clients can just depend on analyzer and don't have to take an explicit dep on a version of package:_fe_analyzer_shared - that package dependency is where we're running into trouble
• if that file is just json, it may be worth in-lining a small parser in package:dwds; it could parse the json and extract out the (few?) fields it needs. You would want some verification there (in the CI for webdev or the SDK CI) that the file stayed in the format that you were expecting

[johnniwinther]

It is not just a json file. In particular with the added support for include tags in https://dart-review.googlesource.com/c/sdk/+/224952, which is intended to solve the long-standing problem of having to update Dart and Flutter repos simultaneously when changing dart: libraries layout, it is not a trivial json file.

[johnniwinther]

cc @sigmundch

[DanTup]

Not resolve dart: URIs. @helin24 would it break IntelliJ? @DanTup would it break dart code?

Not critically, but it would break some functionality. These APIs (being able to map between dart:foo and a relative SDK path) fix issues like flutter/flutter#27189. Without it, users would:

• not be able to use the analyzers Go-to-Definition to jump into dart: sources and set breakpoints before starting debugging
• be shown duplicate source files for dart: libraries if they step into them during debugging (each debug session would produce its own copy of the file, downloaded from the VM, in addition to the analyzer-navigated version - and each would have its own set of breakpoints)

We've done without this for a long time, but it does feel like a limitation that shouldn't exist. I've been looking forward to finally having it resolved as VS Code moves over to the SDK DAPs that use this 🙂.

[elliette]

It sounds like there are a lot of options available, which is good!

Regarding Devon's suggestion:

"to have package:analyzer export the LibrariesSpecification class from the package:_fe_analyzer_shared/src/util/libraries_specification.dart library. Then clients can just depend on analyzer and don't have to take an explicit dep on a version of package:_fe_analyzer_shared - that package dependency is where we're running into trouble"

This seems like it would be a pretty simple change, and wouldn't regress any current functionality. @johnniwinther I'm guessing you have the most context here - are there any downsides to doing this? Thanks!

[johnniwinther]

Given that the LibrariesSpecification class is made for a different purpose, we should expose this functionality by creating a new interface/class in that directly support this use case and then implement this using LibrariesSpecification internally.

[helin24]

Does "depend on analyzer" mean that IDEs using the APIs to resolve URIs would require the analysis server to be running, or does this mean something else? We are moving to using these APIs in IntelliJ to avoid a dependency on the analysis server while setting breakpoints, since users sometimes don't have the analysis server running correctly (especially bazel) and this leads to being unable to set breakpoints. (makes sense now, thanks!)

[annagrin]

I played with flutter breakpoints inside the sdk and realized that they currently do not work on web platform. I suspect the Uris do not resolve correctly or dwds cannot map them to source. I will open a bug to investigate.