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

make bootstrap-sass resolving configurable #254

Closed
vjpr opened this issue Feb 13, 2017 · 1 comment
Closed

make bootstrap-sass resolving configurable #254

vjpr opened this issue Feb 13, 2017 · 1 comment

Comments

@vjpr
Copy link

vjpr commented Feb 13, 2017

Currently bootstrap-sass needs to be resolvable from bootstrap-loader/lib/utils/resolveModule.js.

This means that bootstrap-sass is a peer dependency on bootstrap-loader.

Custom module systems such as pnpm will not be able to find peer deps if they are not explicitly specified in the package.json.

So firstly, it should be declared as a peer dependency. But because you need bootstrap or bootstrap-sass depending on the version, this would create warning messages.

A better approach might be to allow specifying the location of bootstrap-sass or make the resolver context configurable - util/resolveModules.js, resolve.baseDir option.

It's currently not possible to override (pass in as absolute value from ?boostrapPath=) as seen here:

  // Resolve `bootstrap` package
  var bootstrapNPMModule = bootstrapVersion === 3 ? 'bootstrap-sass' : 'bootstrap';

  _logger2.default.debug('Using Bootstrap module:', bootstrapNPMModule);

  config.bootstrapPath = (0, _resolveModule2.default)(bootstrapNPMModule);

...and the config option is not parsed in bootstrap.config.js.


FYI: The use case is if you wanted to create a module which contained all bootstrap related stuff to reduce number of deps declared in your app package.json (instead of having to install bootstrap-sass and all the loaders manually).

E.g.

app
  - node_modules
    - webpack-stuff
      - node_modules
        - bootstrap-loader
        - bootstrap-sass
        - style-loader
        - ...
@justin808
Copy link
Member

@vjpr Thanks for creating the PR. We should be able to get the merged and shipped soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants