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

fix: proxy non-promised methods in browser-snippet #591

Merged
merged 4 commits into from
Sep 26, 2023

Conversation

justin-fiedler
Copy link
Contributor

@justin-fiedler justin-fiedler commented Sep 22, 2023

Summary

AMP-85112

Non-promised functions are not being properly queued in the Browser snippet. There is existing logic in browser-client.ts, but I don't think that works in the proxy case.

<script>
// load snippet function
!function(...)

amplitude.init()
<script>
<script>
   assertFalse(amplitude.invoked)
   amplitude.setUserId("user-1")

   setTimeout(() => {
      assertTrue(amplitude.invoked)
      const userId = amplitude.getUserId()
      // userId is undefined
   }, 3000)
<script>

I added new logic in the snippet to proxy non-promise methods.

@kevinpagtakhan
Copy link
Collaborator

There is existing logic in browser-client.ts, but I don't think that works in the proxy case.

This logic is added for the situation where the SDK is already loaded, but operations are called before init.

@justin-fiedler justin-fiedler marked this pull request as ready for review September 22, 2023 23:22
Copy link
Collaborator

@kevinpagtakhan kevinpagtakhan left a comment

Choose a reason for hiding this comment

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

🚀

packages/analytics-browser/generated/amplitude-snippet.js Outdated Show resolved Hide resolved
@justin-fiedler justin-fiedler merged commit b76a189 into main Sep 26, 2023
3 checks passed
@justin-fiedler justin-fiedler deleted the AMP-85112-setUserId-is-not-queued-in-snippet branch September 26, 2023 20:06
justin-fiedler added a commit that referenced this pull request Sep 26, 2023
* fix: proxy non-promised methods in browser-snippet

* fix: removed extra semicolon in browser-snippet template

* fix: revert manual generated file changes

* chore: undo manual changes to generated snippet
justin-fiedler added a commit that referenced this pull request Sep 27, 2023
* fix: proxy non-promised methods in browser-snippet (#591)

* fix: proxy non-promised methods in browser-snippet

* fix: removed extra semicolon in browser-snippet template

* fix: revert manual generated file changes

* chore: undo manual changes to generated snippet

* fix: update README.md for lerna to pickup package changes
@justin-fiedler justin-fiedler self-assigned this Sep 29, 2023
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.

3 participants