Skip to content

fix: refactor allSettled to remove dependencies #746

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

Merged

Conversation

KonstantinosKoulaxizis
Copy link
Contributor

Remove allSettled package method and use a local one with the same functionality.

allSettled method was throwing Unhandled promise rejection [RangeError: Maximum call stack size exceeded.]

@oscb
Copy link
Contributor

oscb commented Jan 26, 2023

@KonstantinosKoulaxizis Thanks for the contribution! I don't quite get a repro on this error but it seems like a fairly small change so if this fixes it let's do it. I refactored the implementation a bit and added tests.

@oscb oscb linked an issue Jan 26, 2023 that may be closed by this pull request
Copy link

@silesky silesky left a comment

Choose a reason for hiding this comment

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

Less dependencies are always good for third-party libs =) (people are understandably touchy about it).

@@ -94,3 +94,36 @@ export const getPluginsWithReset = (timeline: Timeline) => {

return eventPlugins;
};

type PromiseResult<T> =
Copy link

@silesky silesky Jan 26, 2023

Choose a reason for hiding this comment

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

This is an excellent typescript definition that seems accurate to the spec.

@oscb oscb changed the title Use local function instead of allSettled package fix: refactor allSettled to remove dependencies Jan 26, 2023
@oscb oscb merged commit 479e468 into segmentio:master Jan 26, 2023
oscb pushed a commit that referenced this pull request Feb 2, 2023
## [@segment/analytics-react-native-v2.11.0](https://github.com/segmentio/analytics-react-native/compare/@segment/analytics-react-native-v2.10.1...@segment/analytics-react-native-v2.11.0) (2023-02-02)

### Features

* add end method to flush policies for cleanup ([#753](#753)) ([96b0893](96b0893))

### Bug Fixes

* prevent locking before setting new user data ([#751](#751)) ([d8439e3](d8439e3))
* refactor allSettled to remove dependencies ([#746](#746)) ([479e468](479e468))
* remove non-required parameters from native calls ([#750](#750)) ([6ebd65e](6ebd65e))
@oscb
Copy link
Contributor

oscb commented Feb 2, 2023

🎉 This PR is included in version @segment/analytics-react-native-v2.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@oscb oscb added the released label Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unhandled promise rejection at every flush call
3 participants