chore: Remove deprecated sass engine argument from Eyeglass constructor.

BREAKING CHANGE: It will now raise an error if you try to pass sass as an argument instead of via the options hash.

packages/eyeglass/CHANGELOG.md

@@ -8,10 +8,12 @@
   asynchronous sass functions.
 * Manually specified modules no longer take precedence over modules discovered
   via node package dependencies. [More Information](https://github.com/linkedin/eyeglass/commit/9d9500abd90414ea9bec7c60465f2bdd42e496ef).
-* The following deprecated properties have been removed:
+* The following deprecated APIs have been removed:
   * `require('eyeglass').Eyeglass`. Instead you should do: `const Eyeglass = require('eyeglass'); new Eyeglass(sassOptions);`
   * `const decorate = require('eyeglass').decorate` - Instead you should do `const decorate = require('eyeglass'); const decoratedOpts = decorate(sassOptions)`.
   * `(new require('eyeglass').Eyeglass()).sassOptions()` - Instead you should do `const Eyeglass = require('eyeglass'); let eyeglass = new Eyeglass(sassOptions); eyeglass.options`.
+  * Passing the node-sass engine to Eyeglass as an argument will now raise an
+    error. Instead pass it to Eyeglass via the `eyeglass.engines.sass` option.
 * The following deprecated options will now cause an error:
   * `root` - Use `eyeglass.root` instead.
   * `httpRoot` - Use `eyeglass.httpRoot` instead.

packages/eyeglass/src/Eyeglass.ts

@@ -11,7 +11,7 @@ import * as fs from "fs-extra";
 import { IEyeglass } from "./IEyeglass";
 import * as packageJson from "package-json";
 import { SassFunction } from "node-sass";
-import { SassImplementation, helpers as sassHelpers } from "./util/SassImplementation";
+import { SassImplementation, helpers as sassHelpers, isSassImplementation } from "./util/SassImplementation";
 import { AsyncImporter } from "node-sass";
 import { UnsafeDict, Dict } from "./util/typescriptUtils";
 import heimdall = require("heimdalljs");
@@ -38,14 +38,17 @@ export default class Eyeglass implements IEyeglass {
   modules: EyeglassModules;
   private onceCache: SimpleCache<true>;
 
-  constructor(options: Opts, deprecatedNodeSassArg?: SassImplementation) {
+  constructor(options: Opts) {
+    if (arguments.length === 2) {
+      _forbidNodeSassArg(arguments[1]);
+    }
     let timer = heimdall.start("eyeglass:instantiation");
     this.onceCache = new SimpleCache<true>();
     try {
       // an interface for deprecation warnings
       this.deprecate = deprecator(options);
 
-      this.options = new Options(options, this.deprecate, deprecatedNodeSassArg);
+      this.options = new Options(options);
       this.assets = new Assets(this, this.options.eyeglass.engines.sass);
       this.modules = new EyeglassModules(
         this.options.eyeglass.root,
@@ -107,6 +110,25 @@ export default class Eyeglass implements IEyeglass {
   }
 }
 
+export function _forbidNodeSassArg(arg: unknown): Pick<EyeglassConfig, "engines"> | void {
+  if (isSassImplementation(arg)) {
+    // throw an error
+    throw new Error([
+      "You may no longer pass `sass` directly to Eyeglass. Instead pass it as an option:",
+      "var options = eyeglass({",
+      "  /* sassOptions */",
+      "  ...",
+      "  eyeglass: {",
+      "    engines: {",
+      "      sass: require('node-sass')",
+      "    }",
+      "  }",
+      "});"
+    ].join("\n  "));
+  }
+}
+
+
 const VERSION_WARNINGS_ISSUED: Dict<boolean> = {};
 
 function checkDependencyConflicts(this: IEyeglass): void {

packages/eyeglass/src/index.ts

@@ -1,6 +1,5 @@
 import { Options as Opts, Config } from "./util/Options";
-import { SassImplementation } from "./util/SassImplementation";
-import EyeglassImpl, { resetGlobalCaches } from "./Eyeglass";
+import EyeglassImpl, { resetGlobalCaches, _forbidNodeSassArg } from "./Eyeglass";
 /* eslint-disable @typescript-eslint/no-namespace, no-inner-declarations, no-redeclare */
 
 interface AdditionalFunctions {
@@ -10,13 +9,16 @@ interface AdditionalFunctions {
 type PublicConstructor =
   typeof EyeglassImpl
   & AdditionalFunctions
-  & ((options: Opts, deprecatedNodeSassArg?: SassImplementation) => Config);
+  & ((options: Opts) => Config);
 
 // This is how we convince typescript that there's an object that is
 // both a constructor and a function that returns options.
 function newOrOptions(): PublicConstructor {
-  const __Eyeglass = function (this: undefined | object, options: Opts, deprecatedNodeSassArg?: SassImplementation): EyeglassImpl | Config {
-    let instance = new EyeglassImpl(options, deprecatedNodeSassArg);
+  const __Eyeglass = function (this: undefined | object, options: Opts): EyeglassImpl | Config {
+    if (arguments.length === 2) {
+      _forbidNodeSassArg(arguments[1]);
+    }
+    let instance = new EyeglassImpl(options);
     if (this) {
       // the implicit this object is thrown away :engineer-shrugging:
       return instance;

packages/eyeglass/src/util/Options.ts

@@ -3,8 +3,7 @@
 import { IOptions as GlobOptions } from "glob";
 import * as path from "path";
 import { ModuleSpecifier } from "../modules/EyeglassModule";
-import { DeprecateFn } from "./deprecator";
-import { isSassImplementation, Options as SassOptions, SassImplementation } from "./SassImplementation";
+import { Options as SassOptions, SassImplementation } from "./SassImplementation";
 import { URI } from "./URI";
 import merge = require("lodash.merge");
 import { Importer, FunctionDeclarations } from "node-sass";
@@ -237,38 +236,13 @@ export default class implements Config {
   eyeglass: EyeglassConfig;
   assetsCache?: (cacheKey: string, lazyValue: () => string) => string;
 
-  constructor(...args: [Options, DeprecateFn, SassImplementation | undefined]) {
-    let config = getSassOptions(...args)
+  constructor(options: Options) {
+    let config = getSassOptions(options)
     this.eyeglass = config.eyeglass; // this makes the compiler happy.
     merge(this, config);
   }
 }
 
-function eyeglassOptionsFromNodeSassArg(arg: SassImplementation | undefined, deprecate: DeprecateFn): Pick<EyeglassConfig, "engines"> | void {
-  if (isSassImplementation(arg)) {
-    // throw a deprecation warning
-    deprecate("0.8.0", "0.9.0", [
-      "You should no longer pass `sass` directly to Eyeglass. Instead pass it as an option:",
-      "var options = eyeglass({",
-      "  /* sassOptions */",
-      "  ...",
-      "  eyeglass: {",
-      "    engines: {",
-      "      sass: require('node-sass')",
-      "    }",
-      "  }",
-      "});"
-    ].join("\n  "));
-
-    // and convert it the correct format
-    return {
-      engines: {
-        sass: arg
-      }
-    };
-  }
-}
-
 function includePathsFromEnv(): Array<string> {
   return normalizeIncludePaths(process.env.SASS_PATH, process.cwd());
 }
@@ -416,15 +390,9 @@ function hasForbiddenOptions(options: Options): options is ForbiddenAssetOptions
 
 function getSassOptions(
   options: Options | undefined,
-  deprecate: DeprecateFn,
-  sassArg: SassImplementation | undefined
 ): Config {
   let sassOptions: Options = options || {};
-  // we used to support passing `node-sass` as the second argument to eyeglass,
-  //  this should now be an options object
-  // if the eyeglassOptions looks like node-sass, convert it into an object
-  // this can be removed when we fully deprecate this support
-  let eyeglassOptions: EyeglassSpecificOptions =  merge({}, eyeglassOptionsFromNodeSassArg(sassArg, deprecate));
+  let eyeglassOptions: EyeglassSpecificOptions = {};
   merge(eyeglassOptions, sassOptions.eyeglass);
 
   // determine whether or not we should normalize URI path separators

packages/eyeglass/test/test_options.js

@@ -2,7 +2,6 @@
 
 var Deprecator = require("../lib/util/deprecator").Deprecator;
 var Eyeglass = require("../");
-var VERSION = Eyeglass.VERSION;
 var assert = require("assert");
 var testutils = require("./testutils");
 var path = require("path");
@@ -135,7 +134,7 @@ describe("options", function() {
 
   describe("deprecated interface", function() {
 
-    it("should warn on eyeglass options not in namespace", function(done) {
+    it("should error on eyeglass options not in namespace", function(done) {
       function forbiddenOptionErrorMessage(option) {
         return [
           "`" + option + "` must be passed into the eyeglass options rather than the sass options:",
@@ -220,12 +219,14 @@ describe("options", function() {
       });
     });
 
-    it("should warn when passing sass engine as argument", function(done) {
-      testutils.assertStderr(function(checkStderr) {
+    it("should error when passing sass engine as argument", function(done) {
+      let errorThrown = true;
+      try {
         var eyeglass = new Eyeglass({}, require("node-sass"));
-        checkStderr([
-          "[eyeglass:deprecation] (deprecated in 0.8.0, will be removed in 0.9.0) " +
-          "You should no longer pass `sass` directly to Eyeglass. Instead pass it as an option:",
+        errorThrown = !eyeglass;
+      } catch(e) {
+        assert.equal(e.message, [
+          "You may no longer pass `sass` directly to Eyeglass. Instead pass it as an option:",
           "  var options = eyeglass({",
           "    /* sassOptions */",
           "    ...",
@@ -235,9 +236,10 @@ describe("options", function() {
           "      }",
           "    }",
           "  });"
-        ].join("\n") + "\n");
-        done();
-      });
+        ].join("\n"))
+      }
+      assert(errorThrown, "Expected error was not thrown.")
+      done();
     });
   });
 });