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

update dependencies, fix css-sass.js #14

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

yogas
Copy link

@yogas yogas commented Feb 20, 2022

Hi,

Fixed package for new NodeJs
Replaced node-sass on sass (now npm install going faster)
Remove inherit dependencie
Add enb-css-preprocessor instead enb

Let up version of the package after pull-request.

Thanks)

package.json Outdated
"vow": "0.4.9",
"inherit": "2.2.2"
"dependencies": {
"enb": "^1.5.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

давай оставим везде жесткие зависимости, без циркумфлекса

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tadatuta , готово

@@ -66,8 +65,7 @@ module.exports = require('enb/lib/build-flow').create()
deferred.resolve(cssResult);
} catch (ex) {
ex = ex instanceof Error ? ex : JSON.parse(ex);

var lines = sassSettings.data.split('\n');
var lines = sassSettings.data.split('\n') || [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sassSettings.data — это же всегда строка? тогда не очень понятно, зачем фоллбек, сплит всегда будет давать массив. или я упускаю какой-то кейс?

Copy link
Author

@yogas yogas Feb 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tadatuta , да это не нужно осталось после дебага, убрал

includePaths: [],
data: ''
}, this._sass);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это изменение не понимаю, в таком случае ведь перестают прокидываться внешние опции?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tadatuta , в перый раз года дебажил sassSettings обёрнутый в inherit возвращал undefined, поэтому решил оставить просто объект.

поправил чтобы sass опции добавлялись в sassSettings через спред оператор

@@ -1,7 +1,6 @@
var sass = require('node-sass');
var sass = require('sass');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можешь, пожалуйста, заодно актуализировать комментарий на 14 строке?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tadatuta , готово

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants