maxmx / bootstrap-stylus

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

Invalid import path in index.styl #109

Closed krzysieki closed 8 years ago

krzysieki commented 8 years ago

Hi

(regarding the newest version 5)

I suppose there are invalid paths in file bootstrap-stylus/bootstrap/index.styl (after installing via bower in bower_components directory)

there should be @import "variables" ...

but there is @import "bootstrap/variables" ...

and build fails

maxmx commented 8 years ago

Fixed by 546fe715685030192bfcac73a7e5cb7e8c78377f

IceOnFire commented 8 years ago

Hi maxmx,

Watch out because the .styl files in https://github.com/maxmx/bootstrap-stylus/tree/master/bootstrap still have the wrong path.

maxmx commented 8 years ago

can you clarify @IceOnFire not sure I'm following.

IceOnFire commented 8 years ago

Sure!

If you look at https://github.com/maxmx/bootstrap-stylus/blob/master/bootstrap/index.styl, it has all @imports looking like "bootstrap/something", while in version 4.0.x it doesn't. I see that with https://github.com/maxmx/bootstrap-stylus/commit/546fe715685030192bfcac73a7e5cb7e8c78377f you corrected the script that builds the .styl files, but maybe you need to update the generated .styl files in the repository too.

krzysieki commented 8 years ago

@IceOnFire is right. There are still incorrect paths. All paths in index.styl should be relative to index.styl :) So if files are in the same folder, "bootstrap/something" shall be changed to "something"

maxmx commented 8 years ago

Yeah, but that doesn't matter since the folder is passed in the paths option of stylus in index.js, hence if you are consuming this lib through npm and requiring it and using it as a stylus middleware, everything works.

I bumped the major version since it could potentially break people's project that were using custom builds.

see https://github.com/maxmx/bootstrap-stylus/pull/81

IceOnFire commented 8 years ago

That's the point, I'm not using bootstrap-styl as a middleware, I'm @import'ing it in my app.styl. My project broke because of this change in the index.styl from version 4.x to 5.x. Is there any problem if you correct the paths? Do you want me to do it for you?

maxmx commented 8 years ago

If you paste the code you are using to build your stylus file here I will tell you where to add the property that will make this work with 5.x.x, otherwise stick to 4.x

IceOnFire commented 8 years ago

I'm sorry I didn't quite explain myself well: bootstrap-styl works fine when files are built with my build scripts, I just found an issue when using the Atom editor and its preview package.

The previewer parses my app.styl file and traverses all its @import's to show a preview of the resulting css. Any previewer or linter that parse the .styl files on the fly will produce an error, so I still think it's better to correct the paths in the .styl files even if doing this doesn't affect builds.

maxmx commented 8 years ago

The change was made because people found that they couldn't use multiple frameworks at the same time in the build pipeline that used @import variables; since the variables file would resolve to the last declared one, hence the @import bootstrap/variables, the directory can be seen as a namespace.

I just looked at the preview module for atom and it seems to support a per-lib config file. Would it be possible to add the bootstrap-styl base path to the config file, there is probably a path property you can change.

On Dec 23, 2015, at 17:31, Matteo Antony Mistretta notifications@github.com wrote:

I'm sorry I didn't quite explain myself well: bootstrap-styl works fine when files are built with my build scripts, I just found an issue when using the Atom editor and its preview package.

The previewer parses my app.styl file and traverses all its @import's to show a preview of the resulting css. Any previewer or linter that parse the .styl files on the fly will produce an error, so I still think it's better to correct the paths in the .styl files even if doing this doesn't affect builds.

— Reply to this email directly or view it on GitHub.