pmowrer / node-sass-json-importer

Allows importing json in sass files parsed by node-sass.
MIT License
162 stars 55 forks source link

Update from version 3.2.0 to latest 4.1.0 breaks build. Undefined scss variables #78

Closed awacode21 closed 4 years ago

awacode21 commented 5 years ago

Hello @pmowrer ,

for my current project we updated the node-sass-json-importer from version 3.2.0 to version 4.1.0 but that causes the build to break with error message of undefined variables. The variables it complains about are normal scss variables within scss files which are not part of any json file (see screenshot). Any idea?

50899264-de6a0380-1411-11e9-8b6d-7844a370d46e

The project setup is build with grunt. Using node-sass und grunt-sass. The relevant part in grunt config is the following:

const jsonImporter = require('node-sass-json-importer');

module.exports = {
    css : {
        options: {
            importer: jsonImporter,
            style: 'nested',
            precision: 8,
            trace: true,
            lineNumbers: true,
            sourceMap: true,
            sourceComments: true
        },
        files : [{
            expand: true,
            cwd: '<%= package.sourceFolder %>/scss/',
            src: ['**/*.scss'],
            dest: '<%= package.buildFolder %>/css/',
            ext: '.css'
        }]
    }
}

It would be unfortunate circumstances if we would be forced to stick with an old version of node-sass-json-importer unable to update.

Best regards, Annick

bjoernsteinborn commented 5 years ago

Same for me with 4.1.0

acramatte commented 5 years ago

Seems that since 4.0.0 and https://github.com/Updater/node-sass-json-importer/pull/71 you need to call the function on the jsonImporter you import/require

e.g.

const jsonImporter = require('node-sass-json-importer');
module.exports = {
    css : {
        options: {
            importer: jsonImporter,
            ...
        },
    }
}

should now be

const jsonImporter = require('node-sass-json-importer');
module.exports = {
    css : {
        options: {
            importer: jsonImporter(),
            ...
        },
    }
}
awacode21 commented 5 years ago

yes, i saw that in documentation and it was the first thing i tried, but when calling the function it marks 'invalid' css which is in fact totally valid!

It seems to have a problem with the json file. But the json structure is absolutely valid json and previous version of node-sass-json-importer could deal with it.

Error: Invalid CSS after "{": expected 1 selector or at-rule, was "{" on line 1 of StaticResources/configuration/layout-config.json from line 2 of StaticResources/scss/settings/_all.scss from line 2 of StaticResources/scss/globals/_all.scss from line 5 of StaticResources/scss/accordion.scss {

Tylerian commented 5 years ago

IMHO this is an issue with #74 and that commit should be reverted in order to fix it.

The reasoning behind this is that sass either expects an object literal with a contentskey in order to load that contents, or an object literal with a filekey to load that file as though it had been imported directly (as seen in docs). But not an object literal containing both file and contents.

@pmowrer would you accept a pull request reverting that behavior?

pmowrer commented 5 years ago

@Tylerian Can both be supported? E.g. if file is present, always use that. Else, use contents.

pmowrer commented 4 years ago

Closed by #83

awacode21 commented 4 years ago

Thanks!