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

fix netlify-cms and sass plugin css extraction #4379

Merged
merged 1 commit into from
Mar 8, 2018

Conversation

erquhart
Copy link
Contributor

@erquhart erquhart commented Mar 5, 2018

Creates a separate ExtractTextPlugin instance for both the Netlify CMS and Sass plugins. We probably need every plugin that uses css extraction to do this.

Fixes #4335.

@gatsbybot
Copy link
Collaborator

gatsbybot commented Mar 5, 2018

Deploy preview for gatsbygram ready!

Built with commit 6e2fb06

https://deploy-preview-4379--gatsbygram.netlify.com

@erquhart erquhart force-pushed the fix-netlify-cms-sass branch 2 times, most recently from ddf0d42 to e2a9505 Compare March 5, 2018 22:49
@KyleAMathews
Copy link
Contributor

TravisCI error

@erquhart
Copy link
Contributor Author

erquhart commented Mar 6, 2018

Hmm doesn't seem related.

@erquhart
Copy link
Contributor Author

erquhart commented Mar 6, 2018

Seeing the same failure locally on master.

@erquhart
Copy link
Contributor Author

erquhart commented Mar 6, 2018

@KyleAMathews failure is to due to #4378, it was merged before tests finished.

@KyleAMathews
Copy link
Contributor

Summary of all failing tests
 FAIL  packages/gatsby-plugin-sass/src/__tests__/gatsby-node.js
  ● gatsby-plugin-sass › encountered a declaration exception
    TypeError: ExtractTextPlugin is not a constructor
      2 | const { cssModulesConfig } = require(`gatsby-1-config-css-modules`)
      3 | 
    > 4 | const extractSass = new ExtractTextPlugin(`styles.css`, { allChunks: true })
      5 | 
      6 | exports.modifyWebpackConfig = ({ config, stage }, options) => {
      7 |   const sassFiles = /\.s[ac]ss$/
      
      at Object.<anonymous> (packages/gatsby-plugin-sass/src/gatsby-node.js:4:21)
      at Suite.Object.<anonymous>.describe (packages/gatsby-plugin-sass/src/__tests__/gatsby-node.js:10:35)
      at Object.<anonymous> (packages/gatsby-plugin-sass/src/__tests__/gatsby-node.js:1:41)

^^^ is what I'm seeing on TravisCI

@KyleAMathews
Copy link
Contributor

master could have a different problem /cc @m-allanson

@m-allanson
Copy link
Contributor

m-allanson commented Mar 6, 2018

master on Travis should be green now, see https://travis-ci.org/gatsbyjs/gatsby/builds

@erquhart
Copy link
Contributor Author

erquhart commented Mar 6, 2018

That's the build, but the test fails.

@erquhart
Copy link
Contributor Author

erquhart commented Mar 6, 2018

That sass plugin error is really strange, the code is definitely correct. I'll take another look in the morning.

@m-allanson
Copy link
Contributor

That's the build, but the test fails.

Oh I see, yep! @erquhart Not that it helps, but I had a poke about at this locally. Like you say, it's strange as it looks like it should work 😕

@erquhart
Copy link
Contributor Author

erquhart commented Mar 6, 2018

@KyleAMathews didn't realize the sass plugin test was mocking the extract plugin module, fixed now.

@KyleAMathews KyleAMathews merged commit 6e12832 into gatsbyjs:master Mar 8, 2018
@ghost ghost removed the review label Mar 8, 2018
@KyleAMathews
Copy link
Contributor

Oops, missed merging this a couple of days ago! 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

Successfully merging this pull request may close these issues.

4 participants