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

feat(*web): switch to latest v9 Firebase JS SDK (compat) #7215

Closed
wants to merge 15 commits into from

Conversation

russellwheatley
Copy link
Member

Description

Explore the possibility of using web v9 compat SDKs.

Related Issues

closes #7041

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.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • I signed the CLA.
  • 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.
  • No, this is not a breaking change.

@google-cla google-cla bot added the cla: yes label Oct 20, 2021
@Salakar Salakar changed the title refactor: update all web example apps with v9 compat SDK feat(*web): switch to latest v9 Firebase JS SDK (compat) Nov 9, 2021
@Salakar
Copy link
Member

Salakar commented Nov 9, 2021

Current known issues:

  • Auth Error messages have changed, are now prefixed with Firebase:
  • Auth updatePhotoURL no longer accepts null - how do you now remove a photo from a user profile if this is still possible?
  • Auth updateDisplayName no longer accepts null - how do you now remove a display name from a user profile if this is still possible?

@Ehesp
Copy link
Member

Ehesp commented Nov 9, 2021

FYI the v9 SDKs do infact accept null, or should:

image

I can do some digging and see if it's an SDK issue.

@Salakar
Copy link
Member

Salakar commented Nov 9, 2021

FYI the v9 SDKs do infact accept null, or should:

image

I can do some digging and see if it's an SDK issue.

@Ehesp could it be the compat layer maybe?

@github-actions
Copy link

github-actions bot commented Nov 9, 2021

Visit the preview URL for this PR (updated for commit e54ef98):

https://flutter-firebase-docs--pr7215-russell-web-compat-dve6v0sl.web.app

(expires Thu, 28 Apr 2022 18:00:25 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@Ehesp
Copy link
Member

Ehesp commented Nov 10, 2021

Confirmed v9 SDK itself allows null to be set, will check the compat layer.

@Ehesp
Copy link
Member

Ehesp commented Nov 10, 2021

And on 9.4.0 (& 9.3.0) compat it also works fine in the browser. Hmm.

@google-cla
Copy link

google-cla bot commented Nov 11, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Nov 11, 2021
@Ehesp
Copy link
Member

Ehesp commented Nov 11, 2021

@googlebot I consent.

@google-cla google-cla bot added cla: yes and removed cla: no labels Nov 11, 2021
@Ehesp
Copy link
Member

Ehesp commented Nov 11, 2021

The errors come through with additional space/periods after the message too:

Firebase: The email address is badly formatted. (auth/invalid-email).

@Ehesp
Copy link
Member

Ehesp commented Dec 2, 2021

Tried all sorts - cannot get null values to be accepted. Going to have to hold off merging this one.

# Conflicts:
#	docs/versions.js
#	packages/cloud_firestore/cloud_firestore/example/web/index.html
#	packages/cloud_functions/cloud_functions/example/web/index.html
#	packages/firebase_analytics/firebase_analytics/example/web/index.html
#	packages/firebase_app_check/firebase_app_check/example/web/index.html
#	packages/firebase_auth/firebase_auth/example/web/index.html
#	packages/firebase_core/firebase_core/example/web/index.html
#	packages/firebase_core/firebase_core_web/lib/src/firebase_sdk_version.dart
#	packages/firebase_database/firebase_database/example/web/index.html
#	packages/firebase_messaging/firebase_messaging/example/web/firebase-messaging-sw.js
#	packages/firebase_messaging/firebase_messaging/example/web/index.html
#	packages/firebase_performance/firebase_performance/example/web/index.html
#	packages/firebase_storage/firebase_storage/example/web/index.html
@Salakar
Copy link
Member

Salakar commented Apr 21, 2022

@russellwheatley @Ehesp - it looks like SDK_VERSION used in tests and internally is now gone, we should update internals and tests to remove usages to get this PR working

@russellwheatley
Copy link
Member Author

Still keeping this as blocked as latest compat versions do not appear to work. Firebase installations-compat script is non-existent. But the e2e tests simply hang as well which might be a result of no installations module. Not sure it's worth pursuing this if we're doing a rewrite of the webs side of things for web v9 integration.

@ChristianEdwardPadilla
Copy link
Contributor

@russellwheatley Has a decision been made on whether to go forward with this upgrade or to wait for a web rewrite? Also, where can I follow along with that kind of discussion? An app in google would like to see this JS dep upgrade happen.

@russellwheatley
Copy link
Member Author

russellwheatley commented May 9, 2022

Hey @ChristianEdwardPadilla, we have decided not to continue with making v9-web-compat since it has some incompatibilities, and instead will focus on refactoring the web implementation to integrate with the latest web v9. We've already started refactoring, and we'll be ramping up development from next week.

@firebase firebase locked and limited conversation to collaborators Jun 9, 2022
@Lyokone Lyokone deleted the @russell/web-compat-v9 branch November 29, 2022 08:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[web] Switch to v9 compat libaries
4 participants