Update to major version 5 #802

mxmason · 2021-06-14T23:16:57Z

Summary

Decouplesnode-sass from this module, and requires users to explicitly set a compiler of their choice.
Upgrades our out-of-date dependencies (which also requires some minor changes to the test suite).
Bumps minimum Node version to 12.

Now that we require users to install their own compilers, it is not strictly necessary for us to bump our minimum Node version, but supporting Node versions that have hit EOL could complicate things in the future. It's worth noting that node-sass has adopted Node 12 as a minimum requirement.

New API

Major version 5 requires users to explicitly set a compiler by passing it to a curried function (per #802 (comment)). Both node-sass and sass compilers are permitted.

For example, in gulpfile.js:

# v4
- const sass = require('gulp-sass');
- const dartSass = require('sass');
- sass.compiler = dartSass;
# v5
+ const sass = require('gulp-sass')(require('sass'));

If the compiler is not set, gulp-sass will throw an error advising the user of this API.

Miscellaneous fixes

These issues relate either to the current version of the project bundling node-sass@4, or to our dependencies currently being outdated.

Issue list

resolves This project's future #801
resolves Doesn't install under nodejs 16 #797
resolves Depends on node-sass #785
resolves Gulp-sass depends on Node-Sass #782
resolves Can't install gulp-sass, node-gyp error despite everything up to date. #780
resolves npm audit: Regular Expression Denial of Service #799
resolves Example has minor incompatibility with Gulp 4. #798
resolves The version of lodash used has a security issue (Prototype Pollution) #775
resolves Vulnerabilities in the latest version 4.1.0 #768

Tasks

Also moves node-sass to devDependencies and peerDependencies. This helps prevent dupes and conflicts in packages using gulp-sass

xzyfer · 2021-06-15T06:48:12Z

Thanks for all this effort. The comment about broad version support was with intention of not needing to bump the major to limit the impact on the ecosystem.

I agree the changes to support versions in node-sass forces our hand here, and we had planned to bump the major in order to make node-sass optional anyway.

index.js

Co-authored-by: Michael Mifsud <xzyfer@gmail.com>

xzyfer · 2021-06-15T06:53:10Z

Can the previously proposed updates the CONTRIBUTING.md and README.md also be ported into this PR?

xzyfer · 2021-06-15T06:55:41Z

Additionally, given we're bumping the major and updating the gulp dependencies, are you to confirm whether these proposed README.md make sense to be ported to this PR also?

Apologies I'm not in touch with gulp anymore.

mxmason · 2021-06-15T06:57:54Z

Thanks for looking at my work! I can give the changes to Readme and Contributing a more thoughtful look tomorrow!

I noticed that the initial API for setting the compiler had users passing an argument, as with gulpSass(compiler). Should I revert to that, or keep the API where users set a compiler prop after defining gulpSass?

xzyfer · 2021-06-15T07:02:49Z

When I used gulp heavily the gulpSass(compiler) was a common pattern so I'd favour that approach unless there's a strong argument against it. I believe that linked commit is fairly close to feature complete, so I'd suggest taking that a starting point for this work.

mxmason

Hey @xzyfer! I've made a couple changes, and identified an open concern I'd like your input on. I also updated the PR description to better surface the changes in this PR. There's a tasklist now – please feel free to tell me if I missed something. I hope to come back tomorrow to update the docs.

Two open questions:

Should we update the test suite to provide better confidence about Dart Sass? See Update to major version 5 #802 (comment)
~~Should we update other dependencies in this repo that were not broken by node-sass or node 12, just to provide better stability?~~ Edit: I went ahead and updated all our dependencies to the latest supported by node 12

mxmason · 2021-06-16T18:19:16Z

index.js

+  if (!compiler || !compiler.render) {
+    const message = new PluginError(
+      PLUGIN_NAME,
+      '\n' +
+      'gulp-sass 5 does not have a default Sass compiler; please set one yourself.\n' +
+      'Both the `sass` and `node-sass` packages are permitted.\n' +
+
+        'For example, in your gulpfile:\n\n' +
+        '  var sass = require(\'gulp-sass\')(require(\'sass\'));\n',
+      { showProperties: false },
+    ).toString();
+    process.stderr.write(`${message}\n`);
+    process.exit(1);
+  }
+  gulpSass.compiler = compiler;
+  return gulpSass;
+};


🔍 Please let me know if I should implement this API some other way!

Edit: This is outdated because a8de2a0 resulted in some formatting changes, but the implementation is the same.

index.js

test/main.js

xzyfer

Fantastic work! I'll try to cut a release in the next couple days once the dust settles from today's gulp-sass and node-sass minor releases.

mxmason · 2021-06-24T15:00:54Z

Thanks, @xzyfer! I still need to update the docs, and I can do that over the next couple days if you want!

xzyfer · 2021-06-24T15:03:19Z

Awesome!

Copied from #694

mxmason · 2021-06-25T08:02:24Z

Hey, @xzyfer I've updated the README file to

document the new API
provide a short migration guide
warn about the EOL of node-fibers (we now suggest switching to renderSync)
use examples written for Gulp 4
include new badges to provide more info about this package

Here's the rendered markdown.

I have not updated the Changelog yet, as I saw that the Changelog consists mostly of your cut releases, and i can't cut those. Let me know if I should do anything else!

Edit: Don't know why I tagged Dylan Manning in this comment at first; apologies!

README.md

xzyfer · 2021-06-25T08:39:27Z

Amazing work!

cyrilverloop · 2021-06-26T18:32:43Z

Hi,

Thanks for this update. Is it possible to import this as an ES module ?
Like : import sass from 'gulp-sass';

mxmason · 2021-06-26T23:22:03Z

Hey @cyrilverloop,

This release can’t be imported as an esmodule. I’m open to working on a release that supports both commonjs and esmodules (if @xzyfer thinks it’d be useful), but that would take a bit of effort.

xzyfer · 2021-06-27T04:30:54Z

Would this also require dart-sass and node-sass adopt esm?

…

On Sun, 27 Jun 2021, 9:22 am EJ Mason, ***@***.***> wrote: Hey @cyrilverloop <https://github.com/cyrilverloop>, This release can’t be imported as an esmodule. I’m open to working on a release that supports both commonjs and esmodules (if @xzyfer <https://github.com/xzyfer> thinks it’d be useful), but that would take a bit of effort. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#802 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAENSWB7GN62JFOURM64FCDTUZOKLANCNFSM46WECMZQ> .

cyrilverloop · 2021-06-27T10:20:43Z

I don't know about dart-sass and node-sass, but using import worked in gulp-sass v4.

cyrilverloop · 2021-06-27T10:23:21Z

Can the compiler be defined after the import ?

mxmason · 2021-06-27T17:18:56Z

I might be wrong about ESM support here! I’ll test it when I’m at my desk. But yes, the compiler can be defined after import.

mae829 · 2021-06-30T23:43:08Z

@cyrilverloop not sure you figured it out but this is my implementation after updating to v5

import dartSass from 'sass';
import gulpSass from 'gulp-sass';
const sass = gulpSass( dartSass );

cyrilverloop · 2021-07-01T08:39:49Z

@mae829 thanks for the import. Your solution works for me.

I recommend to put it in the documentation.

mxmason · 2021-07-01T17:36:37Z

Thank you, @mae829! I agree that this should be in the docs and I will make a PR about it soon.

mxmason added 3 commits June 7, 2021 16:05

Update node-sass to 6.0.0

a980945

Also moves node-sass to devDependencies and peerDependencies. This helps prevent dupes and conflicts in packages using gulp-sass

Make mocha test compatible with gulp@4

169c5c2

Update package version, engines

bed1e99

mxmason mentioned this pull request Jun 14, 2021

Bump node-sass to v6.0.0 #795

Closed

xzyfer reviewed Jun 15, 2021

View reviewed changes

index.js Outdated Show resolved Hide resolved

mxmason and others added 5 commits June 14, 2021 23:50

Remove node-sass as peer dependency

8637494

Replace through2 with transfob

2f668da

Uninstall through2

89d6b7c

Throw error if compiler not set

af7eb72

Fix casing of Sass

93b363d

Co-authored-by: Michael Mifsud <xzyfer@gmail.com>

mxmason added 2 commits June 15, 2021 00:32

Use function arg to set compiler, use transfob package

6e82e25

Update test to new API signature

b47ca49

mxmason commented Jun 16, 2021

View reviewed changes

mxmason requested a review from xzyfer June 16, 2021 18:25

mxmason changed the title ~~Enable Node 16 support~~ Update to major version 5 Jun 16, 2021

mxmason added 2 commits June 16, 2021 12:25

Update package versions

0ccfa8b

Fix eslint errors

a8de2a0

mxmason mentioned this pull request Jun 16, 2021

Upgrade lodash to fix a security issue #776

Merged

xzyfer approved these changes Jun 24, 2021

View reviewed changes

mxmason added 6 commits June 24, 2021 21:00

Incorporate previously proposed documentation changes

dd75219

Copied from #694

Update document outline, apply copyedits

921f4b0

Copyedit: overall clarifications

a5345a1

Call out migration guide, update badges

f02c1c5

Document Gulp 4 usage

30b3748

smol clarification

76a8d8c

mxmason requested a review from xzyfer June 25, 2021 07:55

mxmason added 2 commits June 25, 2021 01:08

Copyedit some headings

b57a94c

Yet Another Copyedit™

ffddd9d

xzyfer reviewed Jun 25, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

bbcac47

xzyfer approved these changes Jun 25, 2021

View reviewed changes

xzyfer merged commit 978b8f6 into dlmanning:master Jun 25, 2021

mxmason deleted the feature/update-node-sass branch June 25, 2021 09:42

mxmason mentioned this pull request Jul 1, 2021

Document usage in ESM modules #805

Merged

ravloony mentioned this pull request Feb 22, 2022

Fix node sass shenanigans on node v16 bitmovin/bitmovin-player-ui#442

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to major version 5 #802

Update to major version 5 #802

mxmason commented Jun 14, 2021 •

edited by xzyfer

Loading

xzyfer commented Jun 15, 2021

xzyfer commented Jun 15, 2021

xzyfer commented Jun 15, 2021 •

edited

Loading

mxmason commented Jun 15, 2021

xzyfer commented Jun 15, 2021

mxmason left a comment •

edited

Loading

mxmason Jun 16, 2021 •

edited

Loading

xzyfer left a comment

mxmason commented Jun 24, 2021

xzyfer commented Jun 24, 2021

mxmason commented Jun 25, 2021 •

edited

Loading

xzyfer commented Jun 25, 2021

cyrilverloop commented Jun 26, 2021

mxmason commented Jun 26, 2021

xzyfer commented Jun 27, 2021 via email

cyrilverloop commented Jun 27, 2021

cyrilverloop commented Jun 27, 2021 •

edited

Loading

mxmason commented Jun 27, 2021

mae829 commented Jun 30, 2021

cyrilverloop commented Jul 1, 2021

mxmason commented Jul 1, 2021

Update to major version 5 #802

Update to major version 5 #802

Conversation

mxmason commented Jun 14, 2021 • edited by xzyfer Loading

Summary

New API

Miscellaneous fixes

Tasks

xzyfer commented Jun 15, 2021

xzyfer commented Jun 15, 2021

xzyfer commented Jun 15, 2021 • edited Loading

mxmason commented Jun 15, 2021

xzyfer commented Jun 15, 2021

mxmason left a comment • edited Loading

Choose a reason for hiding this comment

mxmason Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

xzyfer left a comment

Choose a reason for hiding this comment

mxmason commented Jun 24, 2021

xzyfer commented Jun 24, 2021

mxmason commented Jun 25, 2021 • edited Loading

xzyfer commented Jun 25, 2021

cyrilverloop commented Jun 26, 2021

mxmason commented Jun 26, 2021

xzyfer commented Jun 27, 2021 via email

cyrilverloop commented Jun 27, 2021

cyrilverloop commented Jun 27, 2021 • edited Loading

mxmason commented Jun 27, 2021

mae829 commented Jun 30, 2021

cyrilverloop commented Jul 1, 2021

mxmason commented Jul 1, 2021

mxmason commented Jun 14, 2021 •

edited by xzyfer

Loading

xzyfer commented Jun 15, 2021 •

edited

Loading

mxmason left a comment •

edited

Loading

mxmason Jun 16, 2021 •

edited

Loading

mxmason commented Jun 25, 2021 •

edited

Loading

cyrilverloop commented Jun 27, 2021 •

edited

Loading