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 from version 3.2.0 to latest 4.1.0 breaks build. Undefined scss variables #78

Closed
awacode21 opened this issue Jan 9, 2019 · 7 comments

Comments

@awacode21
Copy link

awacode21 commented Jan 9, 2019

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
Copy link

Same for me with 4.1.0

@acramatte
Copy link

Seems that since 4.0.0 and #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
Copy link
Author

awacode21 commented Aug 7, 2019

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
Copy link

Tylerian commented Aug 26, 2019

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
Copy link
Owner

pmowrer commented Oct 24, 2019

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

pmowrer added a commit that referenced this issue Feb 4, 2020
…tly"

Reverts feature adding `fileName` field to the returned object, because
[returning both `fileName` and `contents` is unexpected per the docs](https://sass-lang.com/documentation/js-api#importer)
breaking the importer, as reported in #78.

This reverts commit 3d34986.
@pmowrer pmowrer mentioned this issue Feb 4, 2020
pmowrer added a commit that referenced this issue Feb 4, 2020
…tly"

Reverts feature adding `fileName` field to the returned object, because
[returning both `fileName` and `contents` is unexpected per the docs](https://sass-lang.com/documentation/js-api#importer)
breaking the importer, as reported in #78.

This reverts commit 3d34986.
pmowrer added a commit that referenced this issue Feb 4, 2020
…tly"

Reverts feature adding `fileName` field to the returned object, because
[returning both `fileName` and `contents` is unexpected per the docs](https://sass-lang.com/documentation/js-api#importer)
breaking the importer, as reported in #78.

This reverts commit 3d34986.
@pmowrer
Copy link
Owner

pmowrer commented Feb 4, 2020

Closed by #83

@pmowrer pmowrer closed this as completed Feb 4, 2020
@awacode21
Copy link
Author

Thanks!

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

No branches or pull requests

5 participants