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: use latest Amplitude Browser 2 SDK (2.11.10) for flutter SDK web #216

Merged
merged 21 commits into from
Jan 8, 2025
Merged

feat: use latest Amplitude Browser 2 SDK (2.11.10) for flutter SDK web #216

merged 21 commits into from
Jan 8, 2025

Conversation

chungdaniel
Copy link
Contributor

@chungdaniel chungdaniel commented Dec 31, 2024 •
edited
Loading

Primary changes:

  • Update web files for using Browser SDK 2 as opposed to maintenance Javascript SDK
    • Update web bridge file
    • Update dart-js interface file
    • Update example app to use new script loader
    • Add web-specific configuration options
    • Session replay, autocapture are not currently supported (autocapture functionality to be investigated later)
  • Use js_interop and js_interop_unsafe rather than package:js/js.dart and package:js/js_util.dart
    • Added minimum flutter version of 3.3 to dependency matrix to support js_interop

Extras:

  • Improved documentation for members of Configuration in configuration.dart
  • Fixed lint issues/linter github actions workflow

Note:

  • No tests have been added for web since dart tests default to the Dart VM platform which does not have access to js libraries (e.g. js_interop, package:js/js.dart)

Jira tickets:

@chungdaniel
WIP: have updated sdk able to track events, using js_interop_unsafe
cd95079
@chungdaniel
WIP: Add a few methods to handleMethodCall
7d28b49
@chungdaniel
WIP: able to initialize amplitude without errors
2ce053f
@chungdaniel
WIP: have created classes for js_interop for configuration, event
4532d58 
Library plugin not yet functional, need to instantiate before passing
Autocapture seems to be occuring when using unified script
@chungdaniel
WIP: removed js_interop classes as they were redundant to dart classes
2e70401 
- Use js_interop_unsafe to map directly to JSObject for interop
- Use script loader to load Amplitude-Typescript as unified script had bugs
- Add some web specific configurations to configuration.dart
@chungdaniel
chore: Update documentation for running example app for web
cbb59aa
@chungdaniel
chore: remove comments, add helper method
ed6ef00
@chungdaniel
chore: update min flutter version in README
2af5022
izaaz
izaaz reviewed Dec 31, 2024
View reviewed changes
lib/amplitude_web.dart Outdated Show resolved Hide resolved
lib/configuration.dart Outdated Show resolved Hide resolved
lib/constants.dart Outdated Show resolved Hide resolved
izaaz
izaaz reviewed Jan 1, 2025
View reviewed changes
lib/configuration.dart Outdated Show resolved Hide resolved
izaaz
izaaz reviewed Jan 1, 2025
View reviewed changes
lib/configuration.dart Outdated Show resolved Hide resolved
Mercy811
Mercy811 requested changes Jan 1, 2025
View reviewed changes
Copy link
Contributor

@Mercy811 Mercy811 left a comment

Choose a reason for hiding this comment

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

Thanks @chungdaniel for supporting Browser 2.x in Flutter. Overall looks good to me. See comments. Can we add update the test for the new configuration options? Thanks

lib/amplitude_web.dart Outdated Show resolved Hide resolved
lib/configuration.dart Outdated Show resolved Hide resolved
lib/configuration.dart Outdated Show resolved Hide resolved
lib/configuration.dart Outdated Show resolved Hide resolved
lib/web/amplitude_js.dart Outdated Show resolved Hide resolved
@chungdaniel chungdaniel mentioned this pull request Jan 2, 2025
Open
@chungdaniel
feat: add optOut for browser SDK
d6ea6c7 
- also change wording for getEvent for PR comment
@chungdaniel
fix: Address PR comments:
0b789c4 
- Add device ID doc for Android
- Make documentation more explicit to show in editor on hover
- Use single `minTimeBetweenSessionsMillis` argument to map to both mobile and web
- Add "offline" to configuration for iOS/Android
@chungdaniel
chore: reword doc for getEvent
2d39e15
@chungdaniel
chore: fix lint issues
4ca04d8
@chungdaniel
chore: add new configuration options to test
59281d1 
also rename minTimeBetweenSessionsMillisForWeb for consistency
@chungdaniel
chore: add new config to test
d2cd976
@chungdaniel
Update swift-lint.yml
a6f5c65 
Lint was failing as maxos-12 has been deprecated https://github.com/amplitude/Amplitude-Flutter/actions/runs/12656972512/job/35270750527
@chungdaniel
Use macos-13
8b270bb 
swiftlint not found, potentially related to xcode version
@chungdaniel
Update swift-lint.yml
cbfd0df
@chungdaniel
remove test key
1c7a8e7
@chungdaniel chungdaniel requested a review from Mercy811 January 7, 2025 19:47
@chungdaniel chungdaniel requested a review from izaaz January 7, 2025 19:47
Mercy811
Mercy811 approved these changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@Mercy811 Mercy811 left a comment

Choose a reason for hiding this comment

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

Thank you Daniel! LGTM

@chungdaniel
Copy link
Contributor Author

chungdaniel commented Jan 8, 2025 •
edited
Loading

I've decided to remove offline from the config for all platforms as the docs seem inconsistent. I've made a ticket to revisit this in the future. https://amplitude.atlassian.net/browse/AMP-121543

@chungdaniel chungdaniel merged commit 3ba2a54 into beta Jan 8, 2025
4 checks passed
@chungdaniel chungdaniel deleted the AMP-97228-flutter-web-4.x branch January 8, 2025 20:10
justin-fiedler pushed a commit that referenced this pull request Jan 8, 2025
@amplitude-sdk-bot
chore(release): 4.0.0-beta.6
2db5c78 
# [4.0.0-beta.6](v4.0.0-beta.5...v4.0.0-beta.6) (2025-01-08)

### Features

* use latest Amplitude Browser 2 SDK (2.11.10) for flutter SDK web ([#216](#216)) ([3ba2a54](3ba2a54))
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 8, 2025

🎉 This PR is included in version 4.0.0-beta.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mercy811 Mercy811 Mercy811 approved these changes

@izaaz izaaz izaaz approved these changes

Assignees
No one assigned
Labels
released on @beta
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chungdaniel @izaaz @Mercy811