Skip to content

Make webpack play better with browserify #42

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
merged 4 commits into from
Mar 30, 2017

Conversation

andyburke
Copy link
Contributor

This change will cause a dead code path that would cause browserify problems to be removed. The dead code path had to do with hot module reloading, which appeared to be turned off, but browserify would still find the require() statement with a weird file path in it.

This change also seems to shave a good 15KB off the dist file by running it through UglifyJS, even without any compression or mangling (probably due to comment removal).

Willing to discuss, but this fixed things so I could use this with my browserify setup.

@mohsen1 mohsen1 self-requested a review January 31, 2017 22:22
@andyburke
Copy link
Contributor Author

@mohsen1 any guidance? I've fixed various configuration issues with updating webpack/karma-webpack/etc, but I'm not sure what's up with the tests now?

@mohsen1
Copy link
Owner

mohsen1 commented Feb 2, 2017

Let me checkout the branch and see

@mohsen1
Copy link
Owner

mohsen1 commented Feb 2, 2017

Funny tests pass on my machine! Not sure what's going on?

I was thinking of switching to yarn for this project. Do you want to do that as well?

@andyburke
Copy link
Contributor Author

I'm not sure I have time to undertake much more work on this for now. These changes make the project work for me in our browserify-based SPA. We're currently using my branch. Would love to get these changes mainlined and switch back to the npm-published version.

@mohsen1
Copy link
Owner

mohsen1 commented Feb 3, 2017

I'll try to merge and publish this soon

@Xstoudi
Copy link

Xstoudi commented Mar 30, 2017

Please, really need this pull request to be merged.

@mohsen1 mohsen1 merged commit 184a3b4 into mohsen1:master Mar 30, 2017
@mohsen1
Copy link
Owner

mohsen1 commented Mar 30, 2017

Released at version 2.0

Let me know if you have any problems

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