Skip to content

Fix imports within library#84

Closed
kaetemi wants to merge 2 commits intofleaflet:masterfrom
nbspou:pr/nbpou-fix-imports
Closed

Fix imports within library#84
kaetemi wants to merge 2 commits intofleaflet:masterfrom
nbspou:pr/nbpou-fix-imports

Conversation

@kaetemi
Copy link
Copy Markdown
Contributor

@kaetemi kaetemi commented Aug 7, 2018

Consistently import library files using relative path.
Don't use package: imports for files within the library as it seems to cause the types to be duplicated in another namespace causing mysterious type mismatch issues (The hashCode of a type from the library itself imported through relative path vs imported using package ends up being different).

@johnpryan
Copy link
Copy Markdown
Collaborator

Can you elaborate on the issue? what do you mean the hashCode is different?

@kaetemi
Copy link
Copy Markdown
Contributor Author

kaetemi commented Aug 7, 2018

For example, if in one file within the same library a class named SomeClass is imported using 'src/some_class.dart', and in another file in that library the same is imported as 'package:mylib/src/some_class.dart'; then in those two files SomeClass.hashCode (the type itself) will give two different values.

@johnpryan
Copy link
Copy Markdown
Collaborator

That sounds like an issue with the SDK, it should be perfectly valid to import from the current package using the "package:" prefix.

Since this sounds like an issue with the SDK, I would want to make sure that 1. Is this the only package that causes the problem? and 2. Can you reproduce the issue using a simple project?

@kaetemi
Copy link
Copy Markdown
Contributor Author

kaetemi commented Aug 8, 2018

Yes, it's an issue with Dart, can easily be reproduced in a small project. In general you won't notice until suddenly dynamic type resolutions or looking up things by type randomly don't work.

Regardless, it's not very clean to import files in two different ways, and it also complicates if someone needs to rename the package for forking purposes.

@kaetemi
Copy link
Copy Markdown
Contributor Author

kaetemi commented Aug 8, 2018

Due to Flutter main() being in /lib/ folder..

c:\source\darttest\type_mismatch>dart bin/main.dart
776166849
278110852
a.runtimeType: AwesomeClass
a.runtimeType.hashCode: 333187867
b.runtimeType: AwesomeClass
b.runtimeType.hashCode: 333187867

c:\source\darttest\type_mismatch>dart lib/type_mismatch.dart
258920452
37514116
a.runtimeType: AwesomeClass
a.runtimeType.hashCode: 253396202
b.runtimeType: AwesomeClass
b.runtimeType.hashCode: 1032280833

https://github.com/nbspou/darttest/tree/master/type_mismatch

@kaetemi
Copy link
Copy Markdown
Contributor Author

kaetemi commented Aug 8, 2018

See dart-lang/sdk#33076, and issues linked from there, it's the issue that used for tracking this problem.

@johnpryan
Copy link
Copy Markdown
Collaborator

johnpryan commented Aug 8, 2018

Thanks for the clarification!

it's not very clean to import files in two different ways

I agree, some developers prefer package: imports and some use relative. I prefer package: imports because it doesn't matter where (in the filesystem) the library is being imported from.

Since this is a stylistic change, and the bug you're referring to has the attention of the Dart team, I'm going to close this.

If we take any action, I would prefer we switch every import to a package: import to stay consistent.

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 this pull request may close these issues.

2 participants