OlegAlexander / lakos

Visualize Dart/Flutter library dependencies in Graphviz dot. Detect dependency cycles.
https://pub.dev/packages/lakos
MIT License
83 stars 7 forks source link

import statements not recognized if packages are referenced #4

Closed gretzki closed 2 years ago

gretzki commented 2 years ago

First of all: Thanks for this great tool! One thing that I observed, when using import package:... statements, those are not picked up by your tool, e.g. import 'package:repository/domain_model/status.dart';

Cheers!

gretzki commented 2 years ago

Hey, @OlegAlexander I just found out that I had to change how package imports are parsed. Maybe I'm doing something wrong in my flutter app, since im new to flutter, but I had to change the following:

In resolve_import.dart I had to change packageFilePathParts[0] = 'lib'; // Replace package name with lib with packageFilePathParts.insert(1, 'lib'); in order to work.

OlegAlexander commented 2 years ago

Hi @gretzki,

Sorry I missed your first post 18 days ago. I'm usually much more responsive.

Dart imports work as follows: When importing files from your own internal package, Dart expects these files to be under the lib folder. So you can import internal packages like this:

import 'package:your_package_name/file1.dart';
import 'package:your_package_name/src/file2.dart';

But what that really means is lib/file1.dart and lib/src/file2.dart where lib is in the same folder as your pubspec.yaml file. I know, it's weird.

You can also use relative paths, but the package: import method is preferred when referring to your own package.

Please keep in mind that Lakos will only visualize internal dependencies. It will not visualize external packages, e.g. package:glob/glob.dart.

Please try the 'package:your_package_name' method above and let me know if that still doesn't work for you.

Thanks!

gretzki commented 2 years ago

Hi @OlegAlexander ,

thanks for the reply. Oddly enough, that's the syntax I'm using and the flutter project compiles; and runs too ;) But somehow Lakos does not pick up those dependencies if not modified as in my PR. I'll take a look at my internal packages to find out, if I did something wrong there.

OlegAlexander commented 2 years ago

That's strange. Could you please create a minimal example project for me so I can reproduce the problem on my end?

gretzki commented 2 years ago

yes, I'll gladly provide an example in the next days.

OlegAlexander commented 2 years ago

Hi @gretzki,

Have you had a chance to make an example project for me to try to reproduce this issue? Or have you solved the issue since then? Thanks.

gretzki commented 2 years ago

Alas, I haven't yet. But I can describe it in a few sentences. The app is partitioned into separate modules - dividing the complexity into smaller independent packages so to say. There's a pubspec.yaml that looks like

dependencies:
  flutter:
    sdk: flutter
  flutter_localizations:
    sdk: flutter

...
  #path dependencies
  shared:
    path: modules/shared

The main.dart file lies under root/lib/ Another package like shared lies under root/modules/shared There lies a pubspec.yaml describing the module, and a lib folder containing its sourcecode.

The modules are imported from within other modules or the main app as follows:

import 'package:shared/di/service_locator.dart';

Hope this helps as a starting point

OlegAlexander commented 2 years ago

Hi @gretzki, I think I understand now. Lakos is designed to graph one package at a time, with only one pubspec.yaml file. It ignores all external packages, even if they are your own. I recommend running Lakos on your main project and each of your subprojects separately.

gretzki commented 2 years ago

Hey @OlegAlexander, but it works fine with this small improvement. Do you plan to support this in the future?

OlegAlexander commented 2 years ago

Hi @gretzki, Actually, I just ran the tests on your pull request and they all passed successfully. If you're saying that this simple change is working for you, and it doesn't break anything else, then there's a good chance I'll merge it. Please give me some time to look at your change more carefully.

OlegAlexander commented 2 years ago

Hi @gretzki, I took a closer look at your pull request and ran the tests again with and without your .insert(1, 'lib') change. Unfortunately, your change breaks the graphs in the test suite. Here are a few comparison images with the correct graphs/metrics on the left and the incorrect graphs/metrics on the right.

lakos.metrics_no_test.png lakos metrics_no_test comparison

args.metrics_no_test.png args metrics_no_test comparison

glob.metrics_no_test.png glob metrics_no_test comparison

Can you please update your pull request so that the code works correctly for both the projects in the test suite and also your project?

OlegAlexander commented 2 years ago

Hello @gretzki, Have you had a chance to update your pull request so that it works correctly for both cases, as outlined above? Thank you.

gretzki commented 2 years ago

Hi @OlegAlexander . Alas, I've not really time to work on this. It was just an idea for a quick fix but if it requires more time, I'll not be able to do it. All my spare time goes into a similar project, but for Swift language instead of Flutter/Dart. You might want to take a look at swiftalyzer.com. Sorry, and thanks for this good tool!

OlegAlexander commented 2 years ago

No problem, @gretzki. I will close this issue, but I've added SwiftAlyzer to my Readme. It looks really cool!