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

Updating some dependencies, part 1 #771

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

gwbischof
Copy link
Contributor

@gwbischof gwbischof commented Sep 26, 2023

I had to run flutter pub upgrade to get it to build for linux.
So this PR is just a flutter pub upgrade
App seems to be working for me correctly on linux.
I will update some of the other deps in follow-up PRs, for example extended_image

@hjiangsu
Copy link
Member

@micahmo I'll leave it to you to perform some testing on this with the upgraded dependencies for Android if thats okay? I'll go ahead and do a general run through on iOS just to double check that nothing immediately shows up!

@micahmo
Copy link
Member

micahmo commented Sep 27, 2023

Before I look at this, are we expecting pubspec.yaml to be part of this PR as well? It looks like we only have .lock. Running flutter pub get doesn't change .lock, but it doesn't feel right for them to be out of sync.

@gwbischof
Copy link
Contributor Author

gwbischof commented Sep 27, 2023

In pubspec.yaml we define version requirements for dependencies.
For example: bloc_concurrency: ^0.2.2. The ^ symbol means that the version must be greater than or equal to 0.2.2 and less than 1.0.

It is important for libraries to be open with their dependency version requirements so that they can be compatible with each other.

But for applications like thunder, you want to pin version requirements so that everyone who builds the project gets identical versions of the dependencies. The pubspec.lock file does this. When you do flutter pub get it uses the versions in the pubspec.lock.

flutter pub upgrade looks at the pubspec.yaml to calculate the latest version for each package that can be used, installs those packages and updates the pubspec.lock file with the lastest versions that are compatible with the pubspec.yaml. So next time someone runs flutter pub get they will get newer versions of the dependencies.

So pubspec.yaml and pubspec.lock are still in sync.

@micahmo
Copy link
Member

micahmo commented Sep 27, 2023

Thanks for the response!

When you do flutter pub get it uses the versions in the pubspec.lock.

Just a point of clarification, I believe flutter pub get is what actually resolves and (if necessary) generates/updates pubspec.lock (according to my best understanding of the docs).

This is probably more of a philosophical question than a technical one, but is there any reason to not also update the packages in pubspec.yaml? For example, cached_network_image resolves to 3.3.0 in pubspec.lock. Is there any reason not to put ^3.3.0 in pubspec.yaml? If the answer is "because that might make Thunder incompatible with some packages that need cached_network_image as a dependency at a lower version", then I guess the philosophy would be to always put the lowest possible in .yaml, like ^3.0.0, right?

Anyway, I know every package management system has its own quirks and tries to solve dependency hell in its own ways. So far, haven't had too many issues with Dart/Flutter's implementation.


Also @hjiangsu I ran through a few tests on Android and everything seems fine to me.

@gwbischof
Copy link
Contributor Author

gwbischof commented Sep 27, 2023

I think flutter pub get would only update the lock file if the package didn't already exist in it, That is what I understand anyway, from this section of the docs:
https://docs.flutter.dev/packages-and-plugins/using-packages#updating-package-dependencies

Hmm.. I don't know the answer, but here are some thoughts:

Thunder is an application and not a library (thunder isn't the dependency of another project), so it shouldn't break anything to make the minimum versions in pubspec.yaml match pubspec.lock. Because flutter pub upgrade picked package versions that are compatible with each other when it upgraded the lock file.

Also because Thunder is an application the point of truth for package versions is the lock file, the main purpose of the yaml file is to generate the lock file. So if we are generating a good lock file, then the yaml file is probably also good.

So from my limited understanding, I don't think it should make a difference to update the yaml versions to match the lock.

I will try to update a couple of things in the yaml in a follow-up PR. For example, the extended_image dependency, I don't think we need to install from @hjiangsu fork anymore, I think they fixed the problem in a newer release.

Hopefully getting all of the dependencies updated will fix some of the issues, and hopefully it fixes more issues that it creates. But I think packages tend to improve with time.

@hjiangsu
Copy link
Member

I would just like to say that I'm learning more about how package resolvers work because of this discussion 😅

I think @gwbischof is correct here in terms of how pubspec.yaml and pubspec.lock work. If the app contains a pubspec.lock, then anyone running flutter pub get will get the package versions specified in the lock file. Otherwise, it generates a lock file but never updates it unless we do it manually (or if we install a new package perhaps)

bloc_concurrency: ^0.2.2. The ^ symbol means that the version must be greater than or equal to 0.2.2 and less than 1.0.

One slight correction here is that if a version is below 1.0.0, then pub will treat the minor version as the constraint. In this case, bloc_concurrency: ^0.2.2 would only accept versions greater than or equal to 0.2.2 and less than 0.3.0.

Anyways, if this looks good, I can go ahead and merge this in! Thanks again @gwbischof and @micahmo 😁

@hjiangsu
Copy link
Member

Another thing - I'm indifferent about updating the pubspec.yaml file to match the versions in the lock file. However, I think it's generally safer to keep those versions as is and only change them when we are upgrading to a major version for a package or if there is a vulnerability with one of the versions of the packages we're using.

@gwbischof
Copy link
Contributor Author

One slight correction here is that if a version is below 1.0.0, then pub will treat the minor version as the constraint. In this case, bloc_concurrency: ^0.2.2 would only accept versions greater than or equal to 0.2.2 and less than 0.3.0.

Ohh interesting, I didn't know that

@hjiangsu hjiangsu merged commit 5f3d3e8 into thunder-app:develop Sep 28, 2023
1 check passed
@gwbischof gwbischof deleted the flutter_pub_upgrade branch December 3, 2023 23:03
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.

None yet

3 participants