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

Pin rollup version to fix tree shaking bugs #2572

Merged
merged 1 commit into from
Feb 4, 2021
Merged

Pin rollup version to fix tree shaking bugs #2572

merged 1 commit into from
Feb 4, 2021

Conversation

matthewp
Copy link
Contributor

@matthewp matthewp commented Feb 4, 2021

Changes

BUG FIX

Recent versions of rollup have started tree shaking by removing bindings while leaving the assignment expressions in the code. This breaks lottie-web or any package which binds to a function like this:

var foo = function bar() {

}.bind(this);

This is being compiled to:

function bar() {

}.bind(this);

There might be other cases where removing the binding is also not safe.

Solution is to pin to the previous working version of rollup.

Testing

Adds a test with the exact scenario.

Docs

N/A

Recent versions of rollup have started tree shaking by removing bindings while leaving the assignment expressions in the code. This breaks `lottie-web` or any package which binds to a function like this:

```js
var foo = function bar() {

}.bind(this);
```

This is being compiled to:

```js
function bar() {

}.bind(this);
```

There might be other cases where removing the binding is also not safe.

Solution is to pin to the previous working version of rollup.
@vercel
Copy link

vercel bot commented Feb 4, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/afy4wptkv
✅ Preview: https://snowpack-git-lottie-web.pikapkg.vercel.app

@FredKSchott
Copy link
Owner

LGTM, +1 on doing this until that issue is fixed

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