Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pub workspaces: "pub deps" returns information for the whole workspace and not the current package #4330

Closed
DanTup opened this issue Jul 22, 2024 · 9 comments · Fixed by #4383
Labels
type-question A question about expected behavior or functionality

Comments

@DanTup
Copy link

DanTup commented Jul 22, 2024

Environment

  • Dart version: Dart SDK version: 3.6.0-edge.0b965a1ac8370935ccefa7ca11076b79a2a73037 (main) (Mon Jul 22 08:59:36 2024 +0000) on "windows_x64"
  • OS kind and version (e.g. "Windows 10, version 1809" or "macOS 12.4"): Windows 11 Pro 10.0.22631 Build 22631
  • Are you using the Chinese community mirror or a corporate firewall? No

Problem

Running dart pub deps in a project that's part of a pub workspace seems to just return the full information for the whole workspace and not only the dependencies for the current project.

Expected behavior

Only information for the current projects dependencies should be returned, the same as if it wasn't part of a workspace.

Actual behavior

All dependencies from the workspace are returned.

image

VS Code uses this output for the dependencies tree, and it filters things based on the kind from the json output. However this seems to be quite incorrect for a Pub workspace, so the projects show some random set of dependencies instead of the actual dependencies.

@sigurdm
Copy link
Contributor

sigurdm commented Jul 29, 2024

I was not aware vscode depended on this. I thought it would be practical to show dependencies for each package in the workspace.

The thing is that there is a single resolution covering all the workspace.

Wondering what's the best path forward. Do we change this behavior going forward? Or add more information

Can you detail more exactly what information VS code needs, and how it is used?
Is it for displaying the direct/dev dependencies?

I guess the biggest issue is the json output - maybe we can add to each dependency information about which package has it as direct dependency... Or what the "current" package is...

@DanTup
Copy link
Author

DanTup commented Jul 29, 2024

We use it to render a tree of dependencies for each package in the workspace, broken down by direct/dev/transitive (and in the tooltip we'll show the shortest path to a dependency for transitive dependencies):

image

I don't think anything should change here for workspaces, I think we want things to just work the same. Ideally, we'd still be able to just run a command for a single project and get back its data (because that way we don't need to teach more code about how workspaces work - we used this command instead of looking in package_config ourselves to try and abstract some of the internals of how Pub works away from the editor).

Would it be possible to get the same info (in the same format) as before? If the default behaviour changes, I could pass different flags - but I'd prefer not to have to change this code to be workspace-aware if possible (IMO the less Dart-Code knows about package resolution, the better, because it has to work across many SDK versions and these kinds of things can change).

@sigurdm
Copy link
Contributor

sigurdm commented Aug 1, 2024

I don't think anything should change here for workspaces.

Problem is that things have already changed in dart 3.5 - I think it is too late to revert this behavior.

Also it is not clear what it means to only report deps for a single package in a workspace. As there will be shared dependencies across the shared dependencies.

If we did #3795 this information would be available directly in package_config.json

@DanTup
Copy link
Author

DanTup commented Aug 1, 2024

Problem is that things have already changed in dart 3.5 - I think it is too late to revert this behavior.

Are workspaces being advertised as ready for users in 3.5? If not, the impact of it changing here may be small. Could we have something back for 3.6? It's fine if it requires another flag or something if you don't want to change the default, but right now I don't think I can get the information I had before.

Also it is not clear what it means to only report deps for a single package in a workspace. As there will be shared dependencies across the shared dependencies.

I'm not sure I understand why this changes anything (at least for what I'm using this for). My project still has a pubspec that defines dependencies and dev_depenencies. Which versions they get is now influenced by something external (the root workspace), but all of the information that applied before applies now - my project has a set of dependencies I might want to browse separately from all of the dependencies from the whole workspace.

If we did #3795 this information would be available directly in package_config.json

Part of the reason for Dart-Code using this command was to protect us from changes to package_config. Dart-Code has to work across many SDK versions and that file feels like a bit of an implementation detail we already know too much about (in fact, that it might change in that issue is a good example of why I don't want to be reading it in the editor).

Although, it's not clear if you're noting this because you're suggesting I use it, or that pub deps (or some variation of) would be able to use it then to provide the info that has been lost here?

@sigurdm
Copy link
Contributor

sigurdm commented Aug 1, 2024

Part of the reason for Dart-Code using this command was to protect us from changes to package_config. Dart-Code has to work across many SDK versions and that file feels like a bit of an implementation detail we already know too much about (in fact, that it might change in that issue is a good example of why I don't want to be reading it in the editor).

package_config is documented here: https://github.com/dart-lang/language/blob/main/accepted/2.8/language-versioning/package-config-file-v2.md and should hopefully only add more information moving forward.

You should be able to get the information by combining package_config.json and reading the pubspec.yaml of package folder (it will give you the names of the direct and dev dependencies). I think that would work back to when package_config.json was introduced.

But we might still want to backtrack here, it is not clear that pub deps have many other consumers - so it would be a shame to break those. And we indeed did not announce workspaces widely...

To me it seems slightly confusing that the output of pub deps depends on current folder, instead of giving a global view of the workspace. But that is mainly opinion and might not count for much if we break functionality.

@jonasfj might have something to add...

@DanTup
Copy link
Author

DanTup commented Aug 1, 2024

To me it seems slightly confusing that the output of pub deps depends on current folder, instead of giving a global view of the workspace.

I think workspace info could be useful, but I don't think that negates that it's useful to know about the deps of just this package. I'm not too concerned about the default behaviour so if using a flag to get the info for the current package seems best (or even a different command), that's fine by me.

@sigurdm sigurdm added the type-question A question about expected behavior or functionality label Aug 22, 2024
@DanTup
Copy link
Author

DanTup commented Sep 11, 2024

Any new thoughts on the above? I wonder if I should do something (for example disable the dependencies tree in workspaces) until it's clearer what the solution here will be.

@sigurdm
Copy link
Contributor

sigurdm commented Sep 12, 2024

I think if each package provides a separate list of dependencies vs. dev-dependencies, then vscode could build a tree (maybe rather a forrest rooted in each workspace package) by following the dependencies and dev_dependencies fields recursively.

For compatibility with older sdks, vscode would have to look at the kind for each dependency to gather if it is a dev- or regular dependency

@xvrh
Copy link

xvrh commented Sep 20, 2024

Flutter also need to distinguish which are the real dependencies of a package (instead of take everything from the package_config):

flutter/flutter#155242

Currently the whole package_config is used to find the plugins: https://github.com/flutter/flutter/blob/2528fa95eb3f8e107b7e28651a61305d960eef85/packages/flutter_tools/lib/src/flutter_plugins.dart#L91 (within a workspace it's incorrect)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-question A question about expected behavior or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants