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 chaining by using correct this reference. #11

Merged
merged 2 commits into from
Jan 30, 2018

Conversation

tinyfly
Copy link
Contributor

@tinyfly tinyfly commented Jan 29, 2018

Switch from ES2015 fat arrow function syntax to an anonymous function so that this refers to the mix object and not an empty object.

I tried v1.0.4 and it still wasn't chainable. I would get an error with this:

TypeError: mix.options(...).webpackConfig(...).babelConfig(...).sass(...).copyDirectory(...).js(...).autoload(...).extract(...).purgeCss(...).sourceMaps is not a function

No matter where I put purgeCss in the chain the next function gets the '... is not a function'. It only works if it is the last one in the chain.

The problem was that the this being returned was referencing an empty object. This PR fixes that.

@sebastiandedeyne
Copy link
Member

Good call, was an oversight of mine.

I'd rather not use this though, could you update the PR to return mix with an arrow function instead?

If I had full control over the code I wouldn't mind this, but since we're patching a function to an existing object, I'd rather not use something so "dynamic".

Since we don't control the `this` reference  we return `mix` itself so we can be reliably predictable.
@tinyfly
Copy link
Contributor Author

tinyfly commented Jan 30, 2018

That makes sense. I've made the change back to an arrow function but returning mix instead.

@sebastiandedeyne sebastiandedeyne merged commit 689c6a4 into spatie:master Jan 30, 2018
@sebastiandedeyne
Copy link
Member

Thanks!

@tinyfly tinyfly deleted the fix-chaining branch January 31, 2018 06:29
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.

None yet

2 participants