search

maxmx / bootstrap-stylus

Port of Bootstrap to Stylus
MIT License
585 stars 113 forks source link

Conflicting module names #105

Closed GCheung55 closed 8 years ago

GCheung55 commented 8 years ago

bootstrap-stylus has conflicting module names because they aren't under bootstrap namespace.

For example, an existing path containing a module named variables could already be established so bootstrap/variables doesn't get loaded in bootstrap/index.

To prevent this kind of conflict, the modules should be a full path to the module like:

...
@import "bootstrap/variables"
@import "bootstrap/mixins"
...

and in bootstrap/mixins/

...
@import "bootstrap/mixins/gradients"
...
GCheung55 commented 8 years ago

@maxmx and @kane-c: any thoughts here?

maxmx commented 8 years ago

This isn't like in sass where the root directory would have been added to the path. Unless I'm mistaken I think all the imports in the index.styl are relative no?

GCheung55 commented 8 years ago

I believe that to be partially incorrect.

Assume the following example path: somePath/mixins.styl

somePath is included or set as a "paths" value via the JS API (https://learnboost.github.io/stylus/docs/js.html#includepath). When bootstrap-stylus index.styl imports mixins, it would instead import somePath/mixins.styl.

If somePath directory was not included or set as a "paths" value then the mixins.styl relative to index.styl would load.

maxmx commented 8 years ago

Right, makes sense.

You can submit a PR, I'll bump the major version since that kind of change would break all current usage of the library.

GCheung55 commented 8 years ago

@maxmx I've already submitted a PR. :D https://github.com/maxmx/bootstrap-stylus/pull/106

GCheung55 commented 8 years ago

@maxmx Anything I can do to move this along?

maxmx commented 8 years ago

Done, just had to test it locally first.

maxmx commented 8 years ago

Thank for the PR!

GCheung55 commented 8 years ago

@maxmx no no, thank you!