feat(gatsby-plugin-sass): Support Dart SASS #10159

sibiraj-s · 2018-11-27T11:37:23Z

Feature: Support Dart SASS.

Hi,

Gatsby uses sass-loader which supports both node-sass and dart-sass. This PR enables users to use sass of his choice.

Motivation

In the current implementation gatsby-plugin-sass sends options directly to sass-loader. hence the following usage should work

 options: {
   implementation: require("sass")
  }

but it throws some error, I am not sure of it yet.

Error Message

error Maximum call stack size exceeded


  RangeError: Maximum call stack size exceeded

  - lodash.js:11714 isLength
    [gatsby-hfc-website]/[lodash]/lodash.js:11714:22

  - lodash.js:11334 isArrayLike
    [gatsby-hfc-website]/[lodash]/lodash.js:11334:31

  - lodash.js:13308 keys
    [gatsby-hfc-website]/[lodash]/lodash.js:13308:14

  - lodash.js:4906
    [gatsby-hfc-website]/[lodash]/lodash.js:4906:21

  - lodash.js:2996 baseForOwn
    [gatsby-hfc-website]/[lodash]/lodash.js:2996:24

  - lodash.js:4880
    [gatsby-hfc-website]/[lodash]/lodash.js:4880:18

  - lodash.js:2846 baseEvery
    [gatsby-hfc-website]/[lodash]/lodash.js:2846:7

  - lodash.js:9133 Function.every
    [gatsby-hfc-website]/[lodash]/lodash.js:9133:14

  - data-tree-utils.js:32 isEmptyObjectOrArray
    [gatsby-hfc-website]/[gatsby]/dist/schema/data-tree-utils.js:32:14

  - data-tree-utils.js:36 _.every
    [gatsby-hfc-website]/[gatsby]/dist/schema/data-tree-utils.js:36:16

  - lodash.js:2847
    [gatsby-hfc-website]/[lodash]/lodash.js:2847:20

  - lodash.js:4911
    [gatsby-hfc-website]/[lodash]/lodash.js:4911:15

  - lodash.js:2996 baseForOwn
    [gatsby-hfc-website]/[lodash]/lodash.js:2996:24

  - lodash.js:4880
    [gatsby-hfc-website]/[lodash]/lodash.js:4880:18

  - lodash.js:2846 baseEvery
    [gatsby-hfc-website]/[lodash]/lodash.js:2846:7

  - lodash.js:9133 Function.every
    [gatsby-hfc-website]/[lodash]/lodash.js:9133:14


error UNHANDLED REJECTION


  RangeError: Maximum call stack size exceeded

  - lodash.js:11714 isLength
    [gatsby-hfc-website]/[lodash]/lodash.js:11714:22

  - lodash.js:11334 isArrayLike
    [gatsby-hfc-website]/[lodash]/lodash.js:11334:31

  - lodash.js:13308 keys
    [gatsby-hfc-website]/[lodash]/lodash.js:13308:14

  - lodash.js:4906
    [gatsby-hfc-website]/[lodash]/lodash.js:4906:21

  - lodash.js:2996 baseForOwn
    [gatsby-hfc-website]/[lodash]/lodash.js:2996:24

  - lodash.js:4880
    [gatsby-hfc-website]/[lodash]/lodash.js:4880:18

  - lodash.js:2846 baseEvery
    [gatsby-hfc-website]/[lodash]/lodash.js:2846:7

  - lodash.js:9133 Function.every
    [gatsby-hfc-website]/[lodash]/lodash.js:9133:14

  - data-tree-utils.js:32 isEmptyObjectOrArray
    [gatsby-hfc-website]/[gatsby]/dist/schema/data-tree-utils.js:32:14

  - data-tree-utils.js:36 _.every
    [gatsby-hfc-website]/[gatsby]/dist/schema/data-tree-utils.js:36:16

  - lodash.js:2847
    [gatsby-hfc-website]/[lodash]/lodash.js:2847:20

  - lodash.js:4911
    [gatsby-hfc-website]/[lodash]/lodash.js:4911:15

  - lodash.js:2996 baseForOwn
    [gatsby-hfc-website]/[lodash]/lodash.js:2996:24

  - lodash.js:4880
    [gatsby-hfc-website]/[lodash]/lodash.js:4880:18

  - lodash.js:2846 baseEvery
    [gatsby-hfc-website]/[lodash]/lodash.js:2846:7

  - lodash.js:9133 Function.every
    [gatsby-hfc-website]/[lodash]/lodash.js:9133:14

But the following works,

const sass = require('sass')

options: {
   implementation: {
     info: sass.info,
     render: sass.render
    }
}

Hence the PR to simplify the process.

NOTE: node-sass is removed from peerDependencies to let user to use sass of their choice

sibiraj-s · 2018-11-27T11:48:19Z

Also, I updated the peerDependency gatsby from alpha to stable version

pieh · 2018-11-27T12:03:24Z

about Maximum call stack size exceeded error: this will happen when we try to construct graphql schema (gatsby-config is used there too) and there are some cyclical references. It might be worth exploring ways to short circuit traversing objects if we hit loop there.

What are pros/cons of using dart-sass - maybe we should just switch to it by default?

sibiraj-s · 2018-11-27T12:13:18Z

When it comes to feature they doesn't differ much. It is just a JS implementation of sass.

http://sass.logdown.com/posts/1022316-announcing-dart-sass

painless maintenance too. In my case I often switch node versions in local environment with nvm. Every time when I do that I have to rebuild node-sass with current node. with dart-sass there is no need to do such.

sibiraj-s · 2018-11-27T12:31:18Z

about Maximum call stack size exceeded error: this will happen when we try to construct graphql schema (gatsby-config is used there too) and there are some cyclical references. It might be worth exploring ways to short circuit traversing objects if we hit loop there.

Okay. Can you Point where the schema is created.

packages/gatsby-plugin-sass/src/gatsby-node.js

pieh · 2018-11-27T12:36:53Z

I'm not sure about removing node-sass from peerDependencies. I think we should still keep it, as it will be used 99% of the time.

sibiraj-s · 2018-11-27T12:42:06Z

I'm not sure about removing node-sass from peerDependencies.

This is how sass-loader is implemented.

When the module is not installed sass-loader will throw some meaningful error. Keeping that will just give warning that node-sass is not installed for the users who uses dart-sass too.

pieh · 2018-11-27T12:45:56Z

Okay. Can you Point where the schema is created.

This isn't exactly graphql schema creation (yet) - it's preparation for it. It get caught in infinite loop in this file: https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/src/schema/data-tree-utils.js where we traverse objects to create "pristine example" of node shape (which later on will be used to actually construct schema). Problem is our traversing object doesn't handle cyclical references like this:

const obj = {
  test: 5
}

obj.cyclical = obj

// when traversing object we will get stuck on visiting nested `cyclical` fields 
// (obj.cyclical.cyclical.cyclical...) eventually resulting in maximum call stack size 
// exceeded error

sibiraj-s · 2018-11-28T13:01:20Z

@pieh I have updated the pr. once #10177 is merged. everything should just work fine. Just the documentation is updated.

Also, node-sass is removed from peerDependency, this is based on how sass-loader works https://github.com/webpack-contrib/sass-loader. Since sass-loader is a direct dependency of the plugin. The errors will be handled by sass-loader itself

pieh · 2018-11-28T23:12:16Z

Also, node-sass is removed from peerDependency, this is based on how sass-loader works webpack-contrib/sass-loader. Since sass-loader is a direct dependency of the plugin. The errors will be handled by sass-loader itself

But sass-loader doesn't install node-sass (see #10087 what it looks like when node-sass is not installed). And also this is a way to note version that is supported by plugin.

Hi, This is a fix for issue as discussed here #10159 (comment). prevent loop if the object is of type function

sibiraj-s · 2018-12-06T05:58:53Z

@pieh I have reverted the node-sass dependency back

pieh

Thanks @sibiraj-s 🙏

* master: (870 commits) fix(graphql-skip-limit): declare `graphql` peer dependency (gatsbyjs#10305) fix(gatsby-plugin-offline): gracefully degrade if appshell isn't precached (gatsbyjs#10329) Service workers note (gatsbyjs#10276) fix(docs): link fixes, podcast addition (gatsbyjs#10332) feat(docs): Create clearer pathways in docs (gatsbyjs#9898) feat(www): Rename community section to creators (gatsbyjs#10312) docs(graphql-reference): clarify filtering using comma/and operator (gatsbyjs#10321) chore(release): Publish feat(gatsby-plugin-sass): Support Dart SASS (gatsbyjs#10159) fix(gatsby-source-drupal): use basic auth credentials to fetch remote files as well. (gatsbyjs#10302) fix(gatsby-source-filesystem): allow empty password for basic auth in createRemoteFileNode (gatsbyjs#10280) docs(gatsby-remark-prismjs): Use Gatsby V2 project structure (gatsbyjs#10059) chore: update link for react-gatsby-firebase-authentication (gatsbyjs#10314) fix(www): Awesome Gatsby sidebar link (gatsbyjs#10313) Add thijs koerselman to creators list (gatsbyjs#10303) chore(release): Publish fix(gatsby-plugin-emotion): allow for React.Fragment shorthand syntax (gatsbyjs#10291) feat(www): Update starter cards (gatsbyjs#10258) Update index.md (gatsbyjs#10307) Update index.md (gatsbyjs#10306) ...

Hi, This is a fix for issue as discussed here gatsbyjs#10159 (comment). prevent loop if the object is of type function

* feat: support dart sass * fix: handle module not found error * chore: update dart-sass * chore: revert peerDependencies

snowman · 2019-05-09T15:52:17Z

// in gatsby-config.js
plugins: [
  {
    resolve: `gatsby-plugin-sass`,
    options: {
      implementation: require("sass"),  // should be "dart-sass" instead, otherwise error
    },
  },
]

feat: support dart sass

6e3408d

pieh reviewed Nov 27, 2018

View reviewed changes

packages/gatsby-plugin-sass/src/gatsby-node.js Outdated Show resolved Hide resolved

fix: handle module not found error

f25ff72

sibiraj-s mentioned this pull request Nov 28, 2018

fix(gatsby): skip functions when inferring schema #10177

Merged

chore: update dart-sass

51e2fb3

pieh pushed a commit that referenced this pull request Dec 1, 2018

fix(gatsby): skip functions when inferring schema (#10177)

64a8844

Hi, This is a fix for issue as discussed here #10159 (comment). prevent loop if the object is of type function

chore: revert peerDependencies

ab29c15

pieh approved these changes Dec 6, 2018

View reviewed changes

pieh merged commit a48843e into gatsbyjs:master Dec 6, 2018

gpetrioli pushed a commit to gpetrioli/gatsby that referenced this pull request Jan 22, 2019

fix(gatsby): skip functions when inferring schema (gatsbyjs#10177)

906059a

Hi, This is a fix for issue as discussed here gatsbyjs#10159 (comment). prevent loop if the object is of type function

gpetrioli pushed a commit to gpetrioli/gatsby that referenced this pull request Jan 22, 2019

feat(gatsby-plugin-sass): Support Dart SASS (gatsbyjs#10159)

583abe0

* feat: support dart sass * fix: handle module not found error * chore: update dart-sass * chore: revert peerDependencies

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gatsby-plugin-sass): Support Dart SASS #10159

feat(gatsby-plugin-sass): Support Dart SASS #10159

sibiraj-s commented Nov 27, 2018 •

edited

Loading

sibiraj-s commented Nov 27, 2018

pieh commented Nov 27, 2018

sibiraj-s commented Nov 27, 2018

sibiraj-s commented Nov 27, 2018

pieh commented Nov 27, 2018

sibiraj-s commented Nov 27, 2018

pieh commented Nov 27, 2018

sibiraj-s commented Nov 28, 2018

pieh commented Nov 28, 2018

sibiraj-s commented Dec 6, 2018

pieh left a comment

snowman commented May 9, 2019

feat(gatsby-plugin-sass): Support Dart SASS #10159

feat(gatsby-plugin-sass): Support Dart SASS #10159

Conversation

sibiraj-s commented Nov 27, 2018 • edited Loading

sibiraj-s commented Nov 27, 2018

pieh commented Nov 27, 2018

sibiraj-s commented Nov 27, 2018

sibiraj-s commented Nov 27, 2018

pieh commented Nov 27, 2018

sibiraj-s commented Nov 27, 2018

pieh commented Nov 27, 2018

sibiraj-s commented Nov 28, 2018

pieh commented Nov 28, 2018

sibiraj-s commented Dec 6, 2018

pieh left a comment

Choose a reason for hiding this comment

snowman commented May 9, 2019

sibiraj-s commented Nov 27, 2018 •

edited

Loading