Broken SASS dependency when using Bower

[oxodesign]

When using bower the reference to video-js-fonts package is inaccurate.

This is the reference now
@import "../../node_modules/videojs-font/scss/icons";

This change was as result of this issue > #3998 This is not working for me when using the package on Ember.js trough bower.

It should either be:
@import "../../../node_modules/videojs-font/scss/icons";

or as it was before:
@import "node_modules/videojs-font/scss/icons";

[gkatsev]

As it was before is impossible because sass dropped support for it, the cwd is no longer added as a path by default.
Adding another ../ to the path would then break npm users and our builds system.

We probably should fix this, but I'm not sure how to best go about it without also breaking other usecases.

[wells]

Bringing in a change like this is breaking on other build tools besides bower/ember.

I'm using node-sass v4.5.0 via a build tool called Laravel Mix, essentially a webpack wrapper. What is the version of node-sass is this package's setup is expecting?

It works with these two possibilities for me as well:

• @import "node_modules/videojs-font/scss/icons";
• @import "../../../node_modules/videojs-font/scss/icons";

You could supply a variable for this import statement so we can override like $icon-font-path and continue on our merry ways.

[wells]

@oxodesign A quick fix for you is to fix your version in your package.json and npm update to that.

"video.js": "5.15.1",

[gkatsev]

I do want to apologize for the issue, when we were fixing it our builds were failing left and right and we didn't think it all the way through to other users who could potentially be affected by this change.

We should fix this. Having the $icon-font-path is probably the best option with having it default to ../../node_modules/videojs-font/scss/icons. As mentioned here, sass removed cwd from the default @import paths, so, node_modules/videojs-font/scss/icons is no longer an option. I'm not sure there's a simple solution to fix this without any user intervention at all.

If someone submits a PR to add a path variable and submit it against the 5.x branch, that would be greatly appreciated. Also, any other ideas are definitely welcome.

[wells]

A PR was made on node-sass to restore the old cwd behavior 11 days ago. It was merged into the currently tagged release, v4.5.0. @xzyfer even added tests to make sure this functionality is not removed again. See the pull request for reference. sass/node-sass#1877

For the sake of it, I just cloned the video.js repo and performed a fresh npm i. To confirm, this installed grunt-sass with v4.5.0 of node-sass.

The old way of referencing @import "node_modules/videojs-font/scss/icons"; is working again when I ran grunt build. While I can understand, you would want to hardcode ../.. for your builds, doing so breaks any time that video.js is saved as a dependency on other projects and the sass files are referenced. This is why I think it would be better to change the @import statement back.

[gkatsev]

Ah, in that case, I can make sure to revert that commit. That's for looking and reporting back that the functionality was restored.

[oxodesign]

@wells thanks, I have already rolled back to 5.15.1 and its working. $icon-font-path its a good suggestion.

@gkatsev you dont need to apologize, such things happens and thats why we are here to test these things and give feedback whenever something is not working.

[gkatsev]

This is fixed.