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.

Sign up for GitHub

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

Jump to bottom

feat(web): remove the dependency on package:js in favor of dart:js_interop #12534

Merged
merged 25 commits into from
Apr 2, 2024
Merged

feat(web): remove the dependency on package:js in favor of dart:js_interop #12534

merged 25 commits into from
Apr 2, 2024

Conversation

Lyokone
Copy link
Contributor

@Lyokone Lyokone commented Mar 26, 2024 •
edited
Loading

Description

Also removing the web package when it wasn't used
Removed tests that were generating mocks since those are not compatible with the new interop and were passthrough tests.

Related Issues

#12466

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.

TarekkMA
TarekkMA suggested changes Mar 26, 2024
View reviewed changes
Copy link
Contributor

@TarekkMA TarekkMA left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Missing js_interop import on some files, and I guess firebase_core_web isn't migrated to js_interop yet

@Lyokone Lyokone mentioned this pull request Mar 26, 2024
Closed
13 tasks
TarekkMA
TarekkMA reviewed Mar 29, 2024
View reviewed changes
tests/integration_test/e2e_test.dart
firebase_database.main();
firebase_crashlytics.main();
firebase_auth.main();
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason for the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have add a race condition with auth for a bit of time now, and I tried this out of curiosity and seems to work better ... Might be just placebo tho 😅

@kevmoo
Copy link
Collaborator

kevmoo commented Apr 1, 2024

Can we get this merged?

@Lyokone Lyokone merged commit d83f632 into master Apr 2, 2024
21 checks passed
@Lyokone Lyokone deleted the feat/removejs branch April 2, 2024 07:51
@Lyokone
Copy link
Contributor Author

Lyokone commented Apr 2, 2024

@kevmoo Merged, will be part of next release that should be out today :)

LocLt-Mobile pushed a commit to guide-inc-org/guide-flutter_fire that referenced this pull request Apr 5, 2024
@Lyokone @LocLt-Mobile
feat(web): remove the dependency on package:js in favor of `dart:js…
4a8b50d 
…_interop` (firebase#12534)

* feat(web): remove the dependency on package:js in favor of dart:js_interop

* fix typing

* fixing more stuffs

* tests?

* tests

* more

* fix typing

* fix

* fix typing

* fixing interop typing

* fixing null safety

* more fix

* database

* tests

* tests?

* fix typing

* test order?

* storage

* tests messaging

* fix firestore boxing

* test

* improve

* test

* final??

* clean example app
LocLt-Mobile pushed a commit to guide-inc-org/guide-flutter_fire that referenced this pull request Apr 5, 2024
@Lyokone @LocLt-Mobile
feat(web): remove the dependency on package:js in favor of `dart:js…
7c0db96 
…_interop` (firebase#12534)

* feat(web): remove the dependency on package:js in favor of dart:js_interop

* fix typing

* fixing more stuffs

* tests?

* tests

* more

* fix typing

* fix

* fix typing

* fixing interop typing

* fixing null safety

* more fix

* database

* tests

* tests?

* fix typing

* test order?

* storage

* tests messaging

* fix firestore boxing

* test

* improve

* test

* final??

* clean example app
LocLt-Mobile pushed a commit to guide-inc-org/guide-flutter_fire that referenced this pull request Apr 5, 2024
@Lyokone @LocLt-Mobile
feat(web): remove the dependency on package:js in favor of `dart:js…
9c805ad 
…_interop` (firebase#12534)

* feat(web): remove the dependency on package:js in favor of dart:js_interop

* fix typing

* fixing more stuffs

* tests?

* tests

* more

* fix typing

* fix

* fix typing

* fixing interop typing

* fixing null safety

* more fix

* database

* tests

* tests?

* fix typing

* test order?

* storage

* tests messaging

* fix firestore boxing

* test

* improve

* test

* final??

* clean example app
LocLt-Mobile pushed a commit to guide-inc-org/guide-flutter_fire that referenced this pull request Apr 5, 2024
@Lyokone @LocLt-Mobile
feat(web): remove the dependency on package:js in favor of `dart:js…
c01b206 
…_interop` (firebase#12534)

* feat(web): remove the dependency on package:js in favor of dart:js_interop

* fix typing

* fixing more stuffs

* tests?

* tests

* more

* fix typing

* fix

* fix typing

* fixing interop typing

* fixing null safety

* more fix

* database

* tests

* tests?

* fix typing

* test order?

* storage

* tests messaging

* fix firestore boxing

* test

* improve

* test

* final??

* clean example app
LocLt-Mobile pushed a commit to guide-inc-org/guide-flutter_fire that referenced this pull request Apr 16, 2024
@Lyokone @LocLt-Mobile
feat(web): remove the dependency on package:js in favor of `dart:js…
0e12d06 
…_interop` (firebase#12534)

* feat(web): remove the dependency on package:js in favor of dart:js_interop

* fix typing

* fixing more stuffs

* tests?

* tests

* more

* fix typing

* fix

* fix typing

* fixing interop typing

* fixing null safety

* more fix

* database

* tests

* tests?

* fix typing

* test order?

* storage

* tests messaging

* fix firestore boxing

* test

* improve

* test

* final??

* clean example app
LocLt-Mobile pushed a commit to guide-inc-org/guide-flutter_fire that referenced this pull request Apr 16, 2024
@Lyokone @LocLt-Mobile
feat(web): remove the dependency on package:js in favor of `dart:js…
1528a36 
…_interop` (firebase#12534)

* feat(web): remove the dependency on package:js in favor of dart:js_interop

* fix typing

* fixing more stuffs

* tests?

* tests

* more

* fix typing

* fix

* fix typing

* fixing interop typing

* fixing null safety

* more fix

* database

* tests

* tests?

* fix typing

* test order?

* storage

* tests messaging

* fix firestore boxing

* test

* improve

* test

* final??

* clean example app
@firebase firebase locked and limited conversation to collaborators May 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TarekkMA TarekkMA TarekkMA approved these changes

@russellwheatley russellwheatley russellwheatley approved these changes

@exaby73 exaby73 exaby73 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Lyokone @kevmoo @TarekkMA @russellwheatley @exaby73