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

Dynamic requiring of pusher libraries. #107

Merged
merged 1 commit into from
Mar 13, 2017

Conversation

tlaverdure
Copy link
Contributor

@tlaverdure tlaverdure commented Mar 13, 2017

By default the pusher-js and pusher/react-native packages are required if the pusher broadcaster is selected. This is causing Webpack to fail if the packages are not installed. I'm opposed to installing additional packages when they are not is use, as that potentially leads to a large bundle size. Webpack as a module bundler does not have access to the window object at build time to determine if these packages should be required or not.

This is the best solution that I could find and it seems to work as intended.

Reference:
https://webpack.github.io/docs/context.html#dynamic-require-rewriting

Closes #40, #52

@mKeRix
Copy link

mKeRix commented Mar 13, 2017

This probably fixes #108 as well!

@taylorotwell taylorotwell merged commit 1e9ecfa into laravel:master Mar 13, 2017
@lukepolo
Copy link

This did not fix for me still getting

Cannot find module 'pusher-js'.

@tlaverdure
Copy link
Contributor Author

Are you using webpack or a different bundler?

@lukepolo
Copy link

lukepolo commented Mar 13, 2017

webpack via gulp laravel-elixir-webpack

@aaronhuisinga
Copy link

I'm having the same issue as @lukepolo on a standard laravel-mix compilation. I had to revert back two versions as if I only reverted one, I had the cannot find pusher-react error that this fixed.

@P9Q
Copy link

P9Q commented Mar 14, 2017

Uncaught Error: Cannot find module 'pusher-js'.

Laravel version 5.4.15

@taylorotwell
Copy link
Member

😦 can I just revert these changes as we obviously don't know how to fix this right now?

@tlaverdure
Copy link
Contributor Author

Well it works for my Webpack configuration, so it does resolve some of the issues, but I think that it's not a 100% fix based on the different JS bundling setups devs may be using. This issue has been existent for a while, since pusher-js was added as a dependency, and now with pusher/react-native added, I feel like a precedent has been established that these 3rd party libs should be distributed with laravel-echo.

To fix I would suggest removing pusher-js and pusher/react-native from this project and documenting that those files be required in the project they are being used before instantiating laravel-echo.

@taylorotwell
Copy link
Member

taylorotwell commented Mar 14, 2017 via email

@tlaverdure
Copy link
Contributor Author

Sure I'll send in a sec.

@P9Q
Copy link

P9Q commented Mar 14, 2017

@tlaverdure My problem solved, as you mentioned here 👉 #110, I just added following line:

window.Pusher = require('pusher-js');

into "resources\assets\js\bootstrap.js" and it's working fine,
Thank you and @taylorotwell 😍 👍 👍 👌

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.

6 participants