Skip to content

fix(firebase_ui_auth): Error added in ProfileScreen in firebase_ui_auth 1.4.0 - #10957

Merged
lesnitsky merged 3 commits into
firebase:masterfrom
zachary-russell:patch-1
May 17, 2023
Merged

fix(firebase_ui_auth): Error added in ProfileScreen in firebase_ui_auth 1.4.0#10957
lesnitsky merged 3 commits into
firebase:masterfrom
zachary-russell:patch-1

Conversation

@zachary-russell

@zachary-russell zachary-russell commented May 12, 2023

Copy link
Copy Markdown
Contributor

Description

firebase_ui_auth throws an error when building for version 1.4.0.:
../../.pub-cache/hosted/pub.dev/firebase_ui_auth-1.4.0/lib/src/screens/profile_screen.dart:393:19: Error: Cannot invoke a non-'const' constructor where a const expression is expected. Try using a constructor or factory that is 'const'. const Row( ^^^

Related Issues

Issue

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

  • [ x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [ x] My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • [ x] All existing and new tests are passing.
  • [x ] I updated/added relevant documentation (doc comments with ///).
  • [x ] The analyzer (melos run analyze) does not report any problems on my PR.
  • [ x] I read and followed the Flutter Style Guide.
  • [ x] I signed the CLA.
  • [ x] I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • [ x] No, this is not a breaking change.

@zachary-russell zachary-russell changed the title Fix Error: ../../.pub-cache/hosted/pub.dev/firebase_ui_auth-1.4.0/lib/src/screens/profile_screen.dart:393:19: Error: Cannot invoke a non-'const' constructor where a const expression is expected. Try using a constructor or factory that is 'const'. const Row( ^^^ Fix Error added in ProfileScreen in firebase_ui_auth 1.4.0 May 12, 2023
@zachary-russell zachary-russell changed the title Fix Error added in ProfileScreen in firebase_ui_auth 1.4.0 fix:(firebase_ui_auth) Error added in ProfileScreen in firebase_ui_auth 1.4.0 May 12, 2023
@zachary-russell zachary-russell changed the title fix:(firebase_ui_auth) Error added in ProfileScreen in firebase_ui_auth 1.4.0 fix(firebase_ui_auth): Error added in ProfileScreen in firebase_ui_auth 1.4.0 May 12, 2023
@lesnitsky

lesnitsky commented May 15, 2023

Copy link
Copy Markdown
Contributor

@zachary-russell analyzer fails with your change

firebase_ui_auth:
Analyzing ....

   info - lib/src/screens/profile_screen.dart:393:13 - Use 'const' with the constructor to improve performance. Try adding the 'const' keyword to the constructor invocation. - prefer_const_constructors

1 issue found.

@zachary-russell

Copy link
Copy Markdown
Contributor Author

@zachary-russell analyzer fails with your change


firebase_ui_auth:

Analyzing ....



   info - lib/src/screens/profile_screen.dart:393:13 - Use 'const' with the constructor to improve performance. Try adding the 'const' keyword to the constructor invocation. - prefer_const_constructors



1 issue found.

I think this issue is what caused the bug in the first place. I'll take a look and see if I can resolve it.

The prefer_const_constructors warning is ignored in this case because the LoadingIndicator widget used inside the children list of the Row does not have a const constructor. This is due to the fact that LoadingIndicator extends PlatformWidget, which doesn't have a const constructor, making it impossible to have a const constructor for LoadingIndicator. Since we can't make the LoadingIndicator constructor const, we have to ignore the warning for the specific Row constructor. This is an acceptable solution in cases like this, where making the constructor const is not feasible or possible.
@zachary-russell

zachary-russell commented May 15, 2023

Copy link
Copy Markdown
Contributor Author

@lesnitsky

The prefer_const_constructors warning is ignored in this case because the LoadingIndicator widget used inside the children list of the Row does not have a const constructor. This is due to the fact that LoadingIndicator extends PlatformWidget, which doesn't have a const constructor, making it impossible to have a const constructor for LoadingIndicator. Since we can't make the LoadingIndicator constructor const, we have to ignore the warning for the specific Row constructor. This is an acceptable solution in cases like this, where making the constructor const is not feasible or possible.

I'm still getting errors in the code:

info - lib/src/screens/profile_screen.dart:396:25 - Use 'const' literals as arguments to constructors of '@immutable' classes. Try adding 'const' before the literal. - prefer_const_literals_to_create_immutables

I don't know exactly how to resolve this and I seem to be going in circles. I'd appreciate a nudge in the right direction here :)

Are you aware of another way to solve it I'm definitely willing to give it a stab.

@russellwheatley

Copy link
Copy Markdown
Member

hey @zachary-russell, thanks for the PR. Do you mind changing it so the const is on children like so: https://github.com/firebase/flutterfire/pull/10963/files. I think that should work but if you could double check it builds on your end that would be great. Thanks 😄

@zachary-russell

zachary-russell commented May 16, 2023

Copy link
Copy Markdown
Contributor Author

Sure @russellwheatley, that's what was rejected on the initial commit I made 😸

I can confirm it works (both commits either make the List<Widget> a const or add in const for the items that aren't loading indicator). This seems to be just a bug on the linting/Melos that is a bit overzealous (or at least I can't figure out a way to fix it, the warnings are circular logic flows).

More than happy to help fix the linting rules as well if you cant point me in the right direction.

@russellwheatley russellwheatley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that ought to do it, thanks! 👍

@bkoznov

bkoznov commented May 23, 2023

Copy link
Copy Markdown

Is there a timetable/schedule for when this will be released to pub.dev ?

@firebase firebase locked and limited conversation to collaborators Jun 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants