skratchdot / react-bootstrap-multiselect

A multiselect component for react (with bootstrap). This is a react port of bootstrap-multiselect.
http://projects.skratchdot.com/react-bootstrap-multiselect/
Other
119 stars 62 forks source link

Cannot resolve 'bootstrap' on dev builds #73

Open IamTheHttp opened 7 years ago

IamTheHttp commented 7 years ago

Using a basic webpack configuration for building a project, i ran the following: npm install react-bootstrap-multiselect

inside my entry point I have only one line: import 'react-bootstrap-multiselect';

When building with webpack, I'm getting the following error: Module not found: Error: Can't resolve 'bootstrap' in 'node_modules/react-bootstrap-multiselect/dist'

installed version for react-bootstrap-multiselect: 2.4.1

If I follow these exact steps when installing 2.2.0 specifically - everything works fine. This was found as a regression in an existing project.

skratchdot commented 7 years ago

I'm not sure why bootstrap is listed as a devDependency in this project if we require it here: https://github.com/skratchdot/react-bootstrap-multiselect/blob/master/dist/index.js#L21

@IamTheHttp - Can you run npm install bootstrap and see if your issue goes away? I'll need to work on a fix for this

IamTheHttp commented 7 years ago

Hey @skratchdot
Installing bootstrap separately resolves the issue.(we've chosen to revert back to 2.2.0 for now anyway)

In version 2.2.0 there's no require to bootstrap at all, and so the code runs fine.

In version 2.4.1 there's some defensive code that's designed to check if bootstrap exists or not: https://github.com/skratchdot/react-bootstrap-multiselect/blob/master/dist/index.js#L29

however requiring a non-existing module(bootstrap) in webpack will fail the whole build - maybe there's a different behavior in browserify, but I'm not familiar with it.

skratchdot commented 7 years ago

We should just start using webpack to build this project IMO. It was created years ago as a dumb wrapper around an existing lib, but needs some updating. I'll try to take a look if I get some time (or pull in a PR if anyone else has time). I don't like the examples currently, because they aren't showing the "right" way to handle selection events, etc. So there's definitely room to make a lot of fixes to this project.

Thanks for the input and links!

IamTheHttp commented 7 years ago

Thanks for your reply @skratchdot

Is there any discussion board / forum / reddit for contributing to this project? Is there any issue tracking with roadmap goals?

I'd like to help out but unfortunately I have little to no experience in open source projects, I wouldn't know where to start.

skratchdot commented 7 years ago

This repo is the correct place to discuss issues and roadmap goals. There isn't currently a roadmap b/c I'm not even using this project (it was created years ago b/c I used the code for a project- but that project isn't being used any longer). I open sourced the code in case any one else could benefit from it.

The only "goal" I have for the project is to spend some time trying to clean it up (perhaps start using webpack) and add some functional or unit tests that might help fix some of the issues people report...

sman591 commented 7 years ago

I commented on the PR, but this bug was introduced in https://github.com/skratchdot/react-bootstrap-multiselect/pull/61

It isn't necessarily related to building with Webpack or another system, rather that the expected behavior of conditionally requiring bootstrap doesn't work the way it was thought to work. It probably needs a try/catch block around the bootstrap require rather than an if/else check.

I have a temporary branch (unstable branch) open that reverts the PR as it also prevents our bootstrap nav dropdown menus from working. I've been meaning to open an issue for it but haven't been able to narrow down why it's happening.

South-Paw commented 7 years ago

Also have experienced this issue, we ended up modifying the source and uploading to an internal npm repo.

changes we made were as follows...


//var BS = require('bootstrap');
var React = require('react');
var objectAssign = require('object-assign');
var getOptions = require('./get-options.js');
var bsMultiselect = require('./bootstrap-multiselect.js');
//var bsDropdown;

// make it play nice when we already have bootstrap dropdown loaded.
//if (typeof BS === 'undefined' || typeof BS.dropdown === 'undefined') {
//  bsDropdown = require('./bootstrap-dropdown.js');
//}
//else {
//  bsDropdown = BS.dropdown;
//}

//$ = bsDropdown.init($);
$ = bsMultiselect.init($);

I'd suggest the best way forward is to tell people that to use this they must have Bootstrap included on their page. The plugin shouldn't be trying to load it adhoc if Bootstrap doesn't exist.