Skip to content

Conversation

@mydea
Copy link
Member

@mydea mydea commented Dec 15, 2022

lodash.debounce uses Function(''), which nextjs interprets as unsafe, and does not work there. See: lodash/lodash#5394

We don't really need that fallback anyhow, global || self should be fine for us.

Eventually, we should get rid of lodash...!

lodash.debounce uses `Function('')`, which nextjs interprets as unsafe, and does not work there. 
See: lodash/lodash#5394

We don't really need that fallback anyhow, `global || self` should be fine for us.
@mydea mydea added the Package: replay Issues related to the Sentry Replay SDK label Dec 15, 2022
@mydea mydea requested review from Lms24, billyvg and lforst December 15, 2022 13:25
@mydea mydea self-assigned this Dec 15, 2022
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this so quickly! Agreed, we should get rid of lodash.

@lforst
Copy link
Contributor

lforst commented Dec 15, 2022

We're probably skipping a bit too many CI steps for a change like this...

@mydea mydea merged commit d9963fc into master Dec 15, 2022
@mydea mydea deleted the fn/avoid-lodash-eval branch December 15, 2022 13:51
@mydea
Copy link
Member Author

mydea commented Dec 15, 2022

We're probably skipping a bit too many CI steps for a change like this...

Well, on packages that do not depend on replay (thus aren't using this), this can't really have an impact, so 🤷‍♂️ (from a CI perspective)

@Lms24
Copy link
Member

Lms24 commented Dec 15, 2022

Hmm a consequence of the Browser export is though that more packages now depend on Replay 😅

@mydea
Copy link
Member Author

mydea commented Dec 15, 2022

good point! I'll add replay to the browser dependency list 👍

mydea added a commit that referenced this pull request Dec 16, 2022
To ensure that CI does not re-use dependencies after #6551.
mydea added a commit that referenced this pull request Dec 16, 2022
To ensure that CI does not re-use dependencies after #6551.
Lms24 added a commit that referenced this pull request Jan 4, 2023
…ation (#6593)

Replace lodash's `debounce` function with a custom, minimal implementation that
- Delays the function invocation by a `wait` time and gate it with a `maxWait` value
- Provides the return value of the invocation for subsequent debounced function calls
- Provides a `flush` and `cancel` function on the debounced function (analogously to lodash).
- Invokes the function _after_ the wait/maxwait time triggered (i.e. on the trailing edge). Lodash allows to choose between the leading and trailing edge, which makes the implementation much more complicated than for us necessary
- Works for functions _without_ parameters. By not supporting args, we can further cut down bundle size. (We might want to revisit this in the future but for now, this should do).

With this change, we can also get rid of the package patch we introduced in #6551 and of the `commonjs()` plugin in our build process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Package: replay Issues related to the Sentry Replay SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants