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

import statements not recognized if packages are referenced #4

Closed
gretzki opened this issue Jan 18, 2022 · 14 comments
Closed

import statements not recognized if packages are referenced #4

gretzki opened this issue Jan 18, 2022 · 14 comments

Comments

@gretzki
Copy link

gretzki commented Jan 18, 2022

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
Copy link
Author

gretzki commented Feb 5, 2022

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.

@gretzki gretzki mentioned this issue Feb 5, 2022
@OlegAlexander
Copy link
Owner

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
Copy link
Author

gretzki commented Feb 5, 2022

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
Copy link
Owner

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

@gretzki
Copy link
Author

gretzki commented Feb 7, 2022

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

@OlegAlexander
Copy link
Owner

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
Copy link
Author

gretzki commented Apr 18, 2022

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
Copy link
Owner

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
Copy link
Author

gretzki commented Apr 20, 2022

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

@OlegAlexander
Copy link
Owner

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 OlegAlexander reopened this Apr 20, 2022
@OlegAlexander
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

gretzki commented May 23, 2022

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants