xzyfer
commented
6 years ago The current behaviour is intended. This package isn't specific to any implementation (i.e. node-sass) and purposely isn't aware of any implementation specific extensions.
By design all bets are off investigate you enter into custom importers.
On Mon., 24 Sep. 2018, 5:01 pm GHMT, notifications@github.com wrote:
PROJECT AND DEPENDENCIES
I have
node v8.11.2 npm v5.8.0
and I am using a create-react-app-typescript https://github.com/wmonk/create-react-app-typescript app with following relevant packages installed:
"dependencies": { "@types/reactstrap": "^6.0.2", "bootstrap": "^4.1.3", "react": "^16.4.2", "react-dom": "^16.4.2", "react-scripts-ts": "2.17.0", "reactstrap": "^6.3.1", ... }, ... "devDependencies": { ... "@types/node": "^10.5.6", "@types/react": "^16.4.7", "@types/react-dom": "^16.0.6", "concurrently": "^4.0.1", "node-sass-chokidar": "^1.3.3", "node-sass-glob-importer": "^5.2.0", "typescript": "^3.0.1" ... }.
My relevant scripts in package.json are:
"start": "concurrently \"npm:watch-css\" \"react-scripts-ts start\"", "build-css": "node-sass-chokidar --importer node_modules/node-sass-glob-importer/dist/cli.js src/ -o src/", "watch-css": "npm run build-css && node-sass-chokidar --watch --recursive --importer node_modules/node-sass-glob-importer/dist/cli.js src/ -o src/"
SASS ARCHITECTURE
I have implemented a 7-1 pattern architecture https://sass-guidelin.es/#the-7-1-pattern for my sass files (inside each folder I only have partial files):
[image: image] https://user-images.githubusercontent.com/10683958/45583395-d224ce00-b897-11e8-9e66-d4705773d940.png
My main.scss file looks like this:
@charset 'UTF-8';
@import 'abstracts/'; @import 'vendors/'; @import 'base/'; @import 'layout/'; @import 'components/'; @import 'pages/'; @import 'themes/*'; @import '_shame';
THE PROBLEM
node-sass https://github.com/sass/node-sass and node-sass-chokidar https://www.npmjs.com/package/node-sass-chokidar uses sass-graph to build a Graph object. There is the option --import https://github.com/sass/node-sass#importer--v200---experimental in node-sass, which allows us to define a custom function as an extension of the importer feature of LibSass https://github.com/sass/libsass. There are different libraries to use as the importer out there, but I am using particularly node-sass-glob-importer https://github.com/maoberlehner/node-sass-magic-importer/tree/master/packages/node-sass-glob-importer, which allows to write glob patterns in sass' import statements. This library works perfectly when compiling sass files without watch mode. This is because the imports in any sass file, are parsed by LibSass and then each import statement goes through this library to check if there are some glob patterns in them, if so, the library sends all corresponding paths out of the glob patterns to LibSass again. So far, so good. In watch mode, node-sass builds a Graph object coming from sass-graph and here is the real problem. A Graph object has an "index" property, which is an object whose keys are the paths of all sass files (partials or not) found under the specified watched directory. The value for each key is an entry with following properties https://github.com/xzyfer/sass-graph/blob/v2.2.4/sass-graph.js#L60:
- imports: all import paths found in this current sass file - importedBy: all paths to the files that import this current sass file - modified: a timestamp with the time this current sass file was modified
For instance, in my sass architecture, the entry object for my _shame.scss file looks like:
{ ... "index": { "~/_shame.scss":{ "imports": [], "importedBy": ["~/main.scss"], "modified": [some_timestamp] } } ... }
since main.scss is the only file that imports this file. This is how node-sass realize when to re-render main.scss, either if it was modified directly, or if any file containing its path in the "importedBy" property was modified. And this is working, of course, I modify _shame.scss and I get in the console that "main.scss" was modified and it is re-compiled again. Now, when I modify any file inside abstracts folder, for instance "abstracts/_variables.scss" partial file, I do not get main.scss re-compiled. So, I checked the entry for this file and it looks like:
... "index": { "~/abstracts/_variables.scss":{ "imports": [], "importedBy": [], "modified": [some_timestamp] } } ...
Why is this happening? I started debugging and I got to this point when sass-graph is adding main.scss file into the Graph:
[image: image] https://user-images.githubusercontent.com/10683958/45602075-d8c55980-b9ed-11e8-8dc1-58cc93313184.png
When parsing import statements at line 69 https://github.com/xzyfer/sass-graph/blob/v2.2.4/sass-graph.js#L69, glob patterns are not resolved by the parse-imports script https://github.com/xzyfer/sass-graph/blob/v2.2.4/parse-imports.js. As you can see in the red circle, all imports are still there with the glob pattern.
Where do it breaks? In line 75 https://github.com/xzyfer/sass-graph/blob/v2.2.4/sass-graph.js#L75, more specifically in line 20 inside resolveSassPath method https://github.com/xzyfer/sass-graph/blob/v2.2.4/sass-graph.js#L20. If we execute the condition the if is evaluating with
scssPath = "[path_start]/abstracts/*"
it will throw an exception, it will be catched but nothing will be done (exception swallowing) https://github.com/xzyfer/sass-graph/blob/v2.2.4/sass-graph.js#L23. Thus, false will be returned as result and in line 76 https://github.com/xzyfer/sass-graph/blob/v2.2.4/sass-graph.js#L76 a continue statement will skip that import (it will do the same for all imports with glob patterns) and only the _shame.scss file (since it has no glob pattern and can be resolved by resolveSassPath method) will be included in the "imports" property of the main.scss file inside the Graph object. So, the final result will be main.scss not taking into consideration that is importing files specified by the glob patterns and each partial file matching the glob patterns not taking into consideration that is imported by main.scss. SOLUTION
In my opinion we should come to a common way to parse import statements in node-sass. If watch mode is not specified, it is a LibSass and the (if specified) custom importer function responsibility. If watch mode is specified, then sass-graph parses the imports (not supporting glob patterns). So, whatever I can do with the custom importer is not supported in sass-graph. A temporary solution would be to modify the parse-imports script https://github.com/xzyfer/sass-graph/blob/v2.2.4/parse-imports.js to support glob patterns when building the graph, or even better, to use the custom importer function passed to node-sass (we should pass it all along to sass-graph).
I volunteer to contribute to fix this, if the rest of the community see this as a real bug.
Opinions, comments and tips are very well welcomed!
Best, Max.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/xzyfer/sass-graph/issues/92, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjZWBSqa0oEqIuEvhu9Z63v7stRWks-ks5ueINHgaJpZM4W2JQT .
GHMT
PROJECT AND DEPENDENCIES
I have
and I am using a create-react-app-typescript app with following relevant packages installed:
My relevant scripts in package.json are:
SASS ARCHITECTURE
I have implemented a 7-1 pattern architecture for my sass files (inside each folder I only have partial files):
My main.scss file looks like this:
THE PROBLEM
node-sass and node-sass-chokidar uses sass-graph to build a Graph object. There is the option --import in node-sass, which allows us to define a custom function as an extension of the importer feature of LibSass. There are different libraries to use as the importer out there, but I am using particularly node-sass-glob-importer, which allows to write glob patterns in sass' import statements. This library works perfectly when compiling sass files without watch mode. This is because the imports in any sass file, are parsed by LibSass and then each import statement goes through this library to check if there are some glob patterns in them, if so, the library sends all corresponding paths out of the glob patterns to LibSass again. So far, so good. In watch mode, node-sass builds a Graph object coming from sass-graph and here is the real problem. A Graph object has an "index" property, which is an object whose keys are the paths of all sass files (partials or not) found under the specified watched directory. The value for each key is an entry with following properties:
- imports: all import paths found in this current sass file - importedBy: all paths to the files that import this current sass file - modified: a timestamp with the time this current sass file was modified
For instance, in my sass architecture, the entry object for my _shame.scss file looks like:
since main.scss is the only file that imports this file. This is how node-sass realize when to re-render main.scss, either if it was modified directly, or if any file containing its path in the "importedBy" property was modified. And this is working, of course, I modify _shame.scss and I get in the console that "main.scss" was modified and it is re-compiled again. Now, when I modify any file inside abstracts folder, for instance "abstracts/_variables.scss" partial file, I do not get main.scss re-compiled. So, I checked the entry for this file and it looks like:
Why is this happening? I started debugging and I got to this point when sass-graph is adding main.scss file into the Graph:
When parsing import statements at line 69, glob patterns are not resolved by the parse-imports script. As you can see in the red circle, all imports are still there with the glob pattern.
Where does it break? In line 75, more specifically in line 20 inside resolveSassPath method. If we execute the condition the if is evaluating with
scssPath = "[path_start]/abstracts/*"it will throw an exception, it will be catched but nothing will be done (exception swallowing). Thus, false will be returned as result and in line 76 a continue statement will skip that import (it will do the same for all imports with glob patterns) and only the _shame.scss file (since it has no glob pattern and can be resolved by resolveSassPath method) will be included in the "imports" property of the main.scss file inside the Graph object. So, the final result will be main.scss not taking into consideration that is importing files specified by the glob patterns and each partial file matching the glob patterns not taking into consideration that is imported by main.scss.
SOLUTION
In my opinion we should come to a common way to parse import statements in node-sass. If watch mode is not specified, it is a LibSass and the (if specified) custom importer function responsibility. If watch mode is specified, then sass-graph parses the imports (not supporting glob patterns). So, whatever I can do with the custom importer is not supported in sass-graph. A temporary solution would be to modify the parse-imports script to support glob patterns when building the graph, or even better, to use the custom importer function passed to node-sass (we should pass it all along to sass-graph).
I volunteer to contribute to fix this, if the rest of the community see this as a real bug.
Opinions, comments and tips are very well welcomed!
Best, Max.