Add SCSS Plugins #663

rayandrew · 2018-06-17T10:56:22Z

Added:

Razzle SCSS Plugin
SCSS Examples
Some unit tests

Add little changes to makeLoaderFinder in razzle-dev-utils.
makeLoaderFinder cannot find loader through object like this:

{
  test: /\.(sa|sc)ss$/,
  use: [
    require.resolve('style-loader'),
	require.resolve('css-loader'),
    ...
  ],
}

const styleLoaderFinder = makeLoaderFinder('style-loader');
config.module.rules.find(styleLoaderFinder); // will return undefined

Not yet implemented:

~~Custom options in SCSS plugins~~

rayandrew · 2018-06-17T10:58:12Z

referencing from issue #662

jaredpalmer · 2018-06-17T17:24:02Z

Looks good. But tests didn’t actually run.

rayandrew · 2018-06-17T17:36:05Z

hmm, i think there's something missing. I cannot reproduce it on my own

rayandrew · 2018-06-17T17:46:13Z

@jaredpalmer, is there any special command to run test?

I have already include test command in scss plugin's package.json

* preserve the full --inspect= flag as passed to yarn start instead of treating it as a boolean * update --inspect docs & comments

jaredpalmer

This is truly awesome! You rock! Just a few nits here and there.

jaredpalmer · 2018-06-18T13:59:02Z

packages/razzle-plugin-scss/.eslintrc

+{
+  "env": {
+    "jest": true,
+    "node": true


Do we need this file in the example?

That's the plugin and I think we need it because eslint screams if i don't put it on eslintrc

jaredpalmer · 2018-06-18T14:00:08Z

packages/razzle-plugin-scss/README.md

@@ -1,3 +1,19 @@
 # razzle-plugin-scss

-This is just a stub. Not yet implemented.
+This package contains a plugin for using SCSS with Razzle


We should probably document accepted filename endings (.scss, .sass)?

Ah I forgot to put it too. I think it would be nice to put extension in document

jaredpalmer · 2018-06-18T14:00:21Z

packages/razzle-plugin-scss/README.md

+## Usage in Razzle Projects
+
+```bash
+yarn add razzle-plugin-scss


jaredpalmer · 2018-06-18T14:02:35Z

packages/razzle-plugin-scss/helpers.js

+  postCssLoaderFinder,
+  resolveUrlLoaderFinder,
+  sassLoaderFinder,
+  styleLoaderFinder,


Should we document in README what loaderFinders are being exported so people can reuse them?

It would be a good idea! I don't know whether some loaders are reusable at first

jaredpalmer · 2018-06-18T14:03:42Z

packages/razzle-plugin-scss/package.json

+    "sass-loader": "^7.0.3"
+  },
+  "devDependencies": {
+    "jest": "20.0.4"


Bump to jest 23

Will upgrade it. But may I ask a question?

"husky": "^0.14.3", "jest": "^22.4.3", "lerna": "2.10.1",

razzle lerna package.json still using jest 22.4.3, should I bump it to 23 or stay in 22.4.3?

jaredpalmer · 2018-06-18T14:04:25Z

packages/razzle-plugin-scss/package.json

+    "mini-css-extract-plugin": "^0.4.0",
+    "node-sass": "^4.9.0",
+    "postcss-flexbugs-fixes": "^3.3.1",
+    "razzle": "^2.2.0",


Razzle is not a dep.

Ah I see, can I make it as dev dependency like typescript-plugin?

jaredpalmer · 2018-06-18T14:04:35Z

packages/razzle-plugin-scss/package.json

+  "dependencies": {
+    "autoprefixer": "^8.6.2",
+    "mini-css-extract-plugin": "^0.4.0",
+    "node-sass": "^4.9.0",


Use node-sass-chokidar

it's much less flaky.

I want too, but sass-loader make it as peer dependencies and using node-sass API.

Are you perhaps successfully combine webpack and node-sass-chokidar together?

Yeah it's just a warning. It works fine. We've used it for almost a year.

jaredpalmer · 2018-06-18T14:10:38Z

packages/razzle-plugin-scss/tests/index.test.js

+
+      it('should add style-loader', () => {
+        const rule = config.module.rules.find(styleLoaderFinder);
+        expect(rule).not.toBeUndefined();


When I have a lot of tests that have exactly the same structure, I prefer to just keep them in an array and just call forEach on them. Helps stay DRY.

For example:

const devLoaderTests = [{ name: '...', loaderFinder: '...' }, { // ... }, { // ... }] describe('dev', () => { devLoaderTests.forEach(item => { it(item.name, () => { expect(config.module.rules.find(item.loaderFinder).not.toBeUndefined()); }) }); });

Ahh sorry for repeating codes. I will fix it!

should I make it recursive?

some tests depends on environment like:

describe('razzle-scss-plugin', () => { describe('when creating a web config', () => { describe('when environment set to development', () => { }); describe('when environment set to production', () => { }); ); describe('when creating a node config', () => { describe('when environment set to development', () => { }); describe('when environment set to production', () => { }); ); );

or better if I do like this:

describe('razzle-scss-plugin', () => { describe('web config on development', () => { }); describe('web config on production', () => { }); describe('node config on development', () => { }); describe('node config on production', () => { }); );

what do you think?

here's what I do in mocha + chai for recursive (will change to suitable tests for razzle):

const template = (strings, ...keys) => (...values) => { const dict = values[values.length - 1] || {}; const result = [strings[0]]; keys.forEach((key, i) => { const value = Number.isInteger(key) ? values[key] : dict[key]; result.push(value, strings[i + 1]); }); return result .join("") .replace(/\s+/g, " ") .trim(); }, const generateTest = (authenticated, credential, templates, tests) => { return _.zipWith(templates, tests, (template, test) => { if (_.isFunction(template)) { it(template({ ...test.props, ...credential }), async () => { if (_.isObject(test)) { if (_.isUndefined(test.payload)) { test.check( await authenticated[test.method]( module.exports.baseUrl(test.url) ) ); } else { test.check( await authenticated[test.method]( module.exports.baseUrl(test.url) ).send(test.payload) ); } } }); } else { describe(template.desc({ role: credential.role }), () => { return module.exports.generateTest( authenticated, credential, template.test, test ); }); } });

const templateTest = [ template`should ${"should"} get list of users if ${"role"} is logged in`, template`should ${"should"} edit users if ${"role"} is logged in`, { desc: template`details of specific user by ${"role"}`, test: [ template`should ${"should"} get details of specific wrong id of user because resource not found`, template`should ${"should"} get details of specific correct id of user` ] } ];

const authenticatedTest = [{ credentials: { username: "administrator", password: "administrator", role: "admin" }, tests: [{ url: "/users", method: `get`, check: result => { expect(result).to.have.status(200); expect(result.body).to.have.ownProperty("data"); expect(result.body.data).to.be.an("array"); expect(result.body).to.have.all.keys(paginateProps); } }, { props: { should: "not" }, url: `/users/${user.id_user}`, payload: user, method: `patch`, check: result => { expect(result).to.have.status(403); expect(result.body.message).to.equal("Forbidden"); expect(result.body.name).to.equal("ForbiddenError"); } }, [{ props: { should: "not" }, url: `/user/${user.wrong_id_user}`, method: `get`, check: result => { expect(result).to.have.status(404); expect(result.body.message).to.equal("User not found."); expect(result.body.name).to.equal("NotFoundError"); } }, { url: `/user/${user.id_user}`, method: `get`, check: result => { expect(result).to.have.status(200); expect(result.body).to.be.an("object"); expect(result.body).to.include.all.keys(userColumns); } }], ... ];

and then generate tests like this

authenticatedTest.forEach(auth => { describe(`authenticated role : ${auth.credentials.role}`, () => { const authenticated = chai.request.agent(routes); before(() => knex.migrate .rollback() .then(() => knex.migrate.latest()) .then(() => knex.seed.run()) .then(() => authenticated .post(`${config.get("routePrefix")}/session`) .send(auth.credentials) ) .then(res => { expect(res).to.have.status(200); expect(res.body).to.have.ownProperty("username"); expect(res.body).to.have.ownProperty("status"); }) ); after(() => authenticated.del(`${config.get("routePrefix")}/session`).then(res => { expect(res).to.have.status(200); expect(res.body).to.be.an("object"); expect(res.body.message).to.equal("Logged out successfully."); }) ); generateTest(authenticated, auth.credentials, templateTest, auth.tests); });

what do you think?

jaredpalmer · 2018-06-18T14:12:11Z

packages/razzle-plugin-scss/tests/index.test.js

@@ -0,0 +1,113 @@
+'use strict';
+
+const createConfig = require('razzle/config/createConfig');


Ignore prior comment. Make razzle a devDep.

jaredpalmer · 2018-06-18T14:13:03Z

packages/razzle-plugin-scss/tests/index.test.js

+      expect(rule).toBeUndefined();
+    });
+  });
+});


Remove yarn.lock file please. It will mess up yarn module hoisting

jaredpalmer · 2018-06-18T14:18:37Z

So I think our less and postcss plugins are going to share A LOT of code with this. Do we think it makes sense to abstract some of this into a razzle-plugin-style package and then use that the core for each one? That is, the less, sass, postcss plugins would really just be sort of shell packages that pass in some configs, while the workhorse would be the razzle-plugin-style or like razzle-style-utils or whatever we want to call it.

* added example razzle + react router 3 * Remove yarn.lock * Fix pkg.json

* Add polka example Serves as a generic express middleware-compatible server example. (related to jaredpalmer/after.js#77) * Remove with-polka/setupTests.js * Remove yarn.lock

rayandrew · 2018-06-18T15:03:17Z

That will be a great idea! It seems postcss is used too many times for sure.
It will be great to make it reusable for other plugins too.

PS: gonna make some commits tomorrow with the fixes

rayandrew · 2018-06-20T07:49:01Z

any thoughts about this @jaredpalmer?

jaredpalmer · 2018-06-21T12:58:36Z

Sorry was at an offsite yesterday. Looking right meow 🐱

jaredpalmer

Almost there. Going to discuss the configuration API with my team. Will ping with updates shortly

jaredpalmer · 2018-06-21T12:59:16Z

examples/with-scss/src/App.scss

@@ -1,5 +1,7 @@
+@import url('https://fonts.googleapis.com/css?family=Open+Sans');


jaredpalmer · 2018-06-21T12:59:22Z

packages/razzle-plugin-scss/README.md

@@ -1,11 +1,11 @@
 # razzle-plugin-scss

-This package contains a plugin for using SCSS with Razzle
+This package contains a plugin for using SCSS/SASS with Razzle


Link to the sass website here?

jaredpalmer · 2018-06-21T13:01:43Z

packages/razzle-plugin-scss/README.md

+```js
+// default options
+const defaultOptions = {


When i show off options. Show an example of how to use with a simple change and shortest possible snippet. Then below show/describe defaults beneath. It's just overwhelming here.

Better if I just delete it? I have already shown the default options beneath the example

jaredpalmer · 2018-06-21T13:06:42Z

packages/razzle-plugin-scss/README.md

+    },
+    plugins: [
+      PostCssFlexBugFixes,


Not sure I'm in love with this API. Let me think about this for a few hours and come back with some ideas.

rayandrew · 2018-06-24T07:36:36Z

any updates or changes @jaredpalmer?

…ead (#696)

jaredpalmer · 2018-08-05T15:05:24Z

Reviewing now. Thanks for the ping

jaredpalmer · 2018-08-05T15:05:50Z

If we can fix conflicts will merge.

rayandrew · 2018-08-05T15:07:48Z

i think i need to rebase from the new master branch. will soon resolve the conflicts

thank you for your response @jaredpalmer!

rayandrew · 2018-08-05T16:09:54Z

resolved the conflict, wanna hear your thoughts @jaredpalmer

ion-willo · 2018-08-24T13:47:08Z

any updates @jaredpalmer?

mzedeler · 2018-08-26T20:32:17Z

I could use this plugin too.

zhangking · 2018-09-14T03:49:14Z

When will the plan be released?

snaerth · 2018-09-24T22:09:39Z

Any news on the release

willian · 2018-10-04T15:37:13Z

I am interested in using this too.

nealoke · 2018-10-05T09:18:04Z

+1

snaerth · 2018-10-21T11:17:48Z

No news yet. @RayAndrews would it be possible for you to provide a razzle.config.js for scss and css modules?

jaredpalmer · 2018-10-21T12:52:28Z

@crosscompile can you review on Monday?

jackjocross · 2018-10-22T14:35:16Z

The PR changes LGTM! I agree this will probably share some code with css and postcss plugins but we can probably add an abstraction around those later.

The history in this PR got a little crazy so I pulled out the relevant changes into #786, @RayAndrews let me know if anything is missing from that PR.

rayandrew · 2018-10-22T15:24:08Z

Looks good to me too!
Maybe we can move discussion for this feature in PR #786 and close this one?

cc: @jaredpalmer @crosscompile

rayandrew added 3 commits June 17, 2018 17:32

add scss plugin and fixes to makeLoaderFinder

47a380c

Merge remote-tracking branch 'upstream/master'

9472365

add readme for scss plugin

7ecc4ba

rayandrew mentioned this pull request Jun 17, 2018

razzle-plugin-scss #662

Closed

fix test regex for including .css

5fd5389

remove jest forceExit

1d75cdf

rayandrew and others added 2 commits June 18, 2018 01:01

add contributors

3ade015

preserve the full --inspect= flag as passed to yarn start (#660)

db7b28b

* preserve the full --inspect= flag as passed to yarn start instead of treating it as a boolean * update --inspect docs & comments

jaredpalmer requested changes Jun 18, 2018

View reviewed changes

davidnguyen11 and others added 2 commits June 18, 2018 10:25

added example razzle + react router 3 (#539)

05d795e

* added example razzle + react router 3 * Remove yarn.lock * Fix pkg.json

Add polka example (#520)

9a58a71

* Add polka example Serves as a generic express middleware-compatible server example. (related to jaredpalmer/after.js#77) * Remove with-polka/setupTests.js * Remove yarn.lock

rayandrew added 3 commits June 19, 2018 20:56

refactor tests

2224770

add plugin options

e4738d0

edit readme for better options knowledge

019ffc0

jaredpalmer requested changes Jun 21, 2018

View reviewed changes

rayandrew and others added 2 commits June 21, 2018 22:37

change node-sass to node-sass-chokidar

4123328

Adding testEnvironmentOptions to supportedKeys for Jest (#678)

8a35624

ambar and others added 4 commits July 5, 2018 18:44

fix koa hmr (#693)

72934d9

Add pretty node errors (#691)

b4f9898

Ignore column from source map and only use pretty errors if file is r…

c490929

…ead (#696)

Add documentation for CSS Modules (#694)

63a8b5c

Fix production sourcemaps (#717)

122f847

rayandrew added 13 commits August 5, 2018 22:19

add scss plugin and fixes to makeLoaderFinder

bf1d1b1

add readme for scss plugin

81d2427

fix test regex for including .css

3c3b2a9

remove jest forceExit

98200be

add contributors

9a7f1b1

refactor tests

ed621d7

add plugin options

4cd758e

edit readme for better options knowledge

24abd0e

change node-sass to node-sass-chokidar

b11fd27

bumped package version and fix conflicts

3cd6361

bumped package version and fix conflicts

fdf4aad

bumped package version and fix conflicts

e017028

edited readme

1b39d1e

jackjocross mentioned this pull request Oct 22, 2018

Feature/scss plugin #786

Merged

jaredpalmer closed this Oct 22, 2018

		@@ -0,0 +1,113 @@
		'use strict';

		const createConfig = require('razzle/config/createConfig');

		@@ -1,5 +1,7 @@
		@import url('https://fonts.googleapis.com/css?family=Open+Sans');

Add SCSS Plugins #663

Add SCSS Plugins #663

Conversation

rayandrew commented Jun 17, 2018 • edited Loading

Added:

Not yet implemented:

rayandrew commented Jun 17, 2018

jaredpalmer commented Jun 17, 2018

rayandrew commented Jun 17, 2018

rayandrew commented Jun 17, 2018

jaredpalmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rayandrew Jun 19, 2018 • edited Loading

Choose a reason for hiding this comment

rayandrew Jun 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaredpalmer commented Jun 18, 2018

rayandrew commented Jun 18, 2018 • edited Loading

rayandrew commented Jun 20, 2018

jaredpalmer commented Jun 21, 2018

jaredpalmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rayandrew commented Jun 24, 2018

jaredpalmer commented Aug 5, 2018

jaredpalmer commented Aug 5, 2018

rayandrew commented Aug 5, 2018

rayandrew commented Aug 5, 2018

ion-willo commented Aug 24, 2018

mzedeler commented Aug 26, 2018

zhangking commented Sep 14, 2018

snaerth commented Sep 24, 2018

willian commented Oct 4, 2018

nealoke commented Oct 5, 2018

snaerth commented Oct 21, 2018

jaredpalmer commented Oct 21, 2018

jackjocross commented Oct 22, 2018

rayandrew commented Oct 22, 2018 • edited Loading

rayandrew commented Jun 17, 2018 •

edited

Loading

rayandrew Jun 19, 2018 •

edited

Loading

rayandrew Jun 19, 2018 •

edited

Loading

rayandrew commented Jun 18, 2018 •

edited

Loading

rayandrew commented Oct 22, 2018 •

edited

Loading