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

jQuery dependency problematic with Webpack #578

Closed
TomiS opened this issue May 24, 2016 · 6 comments
Closed

jQuery dependency problematic with Webpack #578

TomiS opened this issue May 24, 2016 · 6 comments

Comments

@TomiS
Copy link
Contributor

TomiS commented May 24, 2016

Hi. I'm trying to wrap a slider inside a React component using Webpack as a module bundler. I don't have jQuery installed at all. Everything works mostly great but there might be a problem with how this library is related to jQuery. Docs say there is no requirement for jQuery but Webpack tries to look for jQuery when bundling the source code and produces this error:

Module not found: Error: Cannot resolve module 'jquery' in /node_modules/bootstrap-slider/dist
 @ ./~/bootstrap-slider/dist/bootstrap-slider.js 42:2-29

As the error states, Webpack ends up here:
https://github.com/seiyria/bootstrap-slider/blob/v7.0.2/dist/bootstrap-slider.js#L42

If I remove the quoted "jquery" dependency like this: define([], factory); the error disappears. I have very little experience with amd wrappers so I'm not sure how this should be solved but I kind of want to question if jQuery really should be a hard dependency on that line of code when it's not a dependency even in package.json

ps. Thanks for the otherwise very nice library.

@rovolution
Copy link
Collaborator

@TomiS thank you for filing a bug report!

I am aware of this and i am hoping to get a fix out sometime soon

@rovolution
Copy link
Collaborator

rovolution commented May 26, 2016

@TomiS were you able to get it to work when you replaced the "jQuery" dependency with an empty string?

From from basic Google searching/Github issue searching/playing around with it in an app using a webpack setup, the best option I can think of for the time being would be to wrap the define() block in a try...catch, which prevents it from being a webpack error and instead displays it as a build warning

Go into the node_modules/dist/bootstrap-slider.js file, change the define() block to the following, rebuild your code, and let me know what happens. You should get the same message, but as a warning instead of an error, which should still allow the code to be built. If it works, we can go ahead and make a PR

if (typeof define === "function" && define.amd) {
        var jQuery;
        try {
            define(["jquery"], factory);
        } catch (err) {
            define([""], factory);
        }
        ...
    }

@TomiS
Copy link
Contributor Author

TomiS commented May 26, 2016

I tried and it doesn't work with an empty string but works with a totally empty array. Also I think var jQuery is unnecessary. I took the liberty of creating a pull request because I agree this modification definitely improves the situation. However, I wouldn't consider this totally solved because the warning is ugly :)

@rovolution
Copy link
Collaborator

However, I wouldn't consider this totally solved because the warning is ugly :)

Agreed. If you end up finding a more elegant solution to this, please let us know! I tried to fruitlessly Google variations of "UMD modules webpack build error" and could not find anything better :(

Thanks for making the PR! I will merge/version bump

@TomiS
Copy link
Contributor Author

TomiS commented May 26, 2016

Thanks for merging and tagging. I'll keep my eyes open for improvements.

@orszaczky
Copy link

Is this a regression? Looks like PR #580 has been merged, and yet the update is not in the codebase:
649128f

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

No branches or pull requests

3 participants