From 57286a7b32aea76e762f67f53f16393195061cfe Mon Sep 17 00:00:00 2001 From: Lenny Burdette Date: Fri, 23 Jun 2017 16:09:25 -0700 Subject: [PATCH 1/2] feat(makeDefaultExport): add deprecation message for generated `default` exports If a module does not define a default export, loader.js assigns `exports.default = exports` for compatibility with modules that don't use `_interopRequireDefault` from babel6. Mutating exports is unexpected behavior and causes difficult-to-diagnose [bugs][1]. We should remove `makeDefaultExport` in version 5.0. This update * adds a deprecation warning in preparation for removing `makeDefaultExport`. * adds a debug build with the deprecation enabled. Addresses #114 [1]:https://github.com/mike-north/ember-lodash/pull/104 --- build.js | 27 ++++++++++++++---- lib/loader/loader.js | 37 +++++++++++++++++++++++- package.json | 1 + tests/all.js | 67 ++++++++++++++++++++++++++++++++++++++++++++ yarn.lock | 8 +++++- 5 files changed, 133 insertions(+), 7 deletions(-) diff --git a/build.js b/build.js index 1ddbd59..3d3cbb9 100755 --- a/build.js +++ b/build.js @@ -5,20 +5,37 @@ var transform = babel.transform; var fs = require('fs'); var mkdirp = require('mkdirp').sync; +var baseTransformPlugins = [ + 'transform-es2015-destructuring', + ['babel-plugin-debug-macros', { + envFlags: { + source: 'env-flags', + flags: { DEBUG: true } + }, + debugTools: { + source: 'debug-tools' + } + }] +]; + mkdirp('./dist/loader'); var source = fs.readFileSync('./lib/loader/loader.js', 'utf8'); + var instrumented = transform(source, { - plugins: ['transform-es2015-destructuring'] + plugins: baseTransformPlugins +}).code; + +var debug = transform(source, { + plugins: baseTransformPlugins }).code; var stripped = transform(source, { // strip-heimdall *must* come before transpiling destructuring plugins: [ - 'babel6-plugin-strip-heimdall', - 'transform-es2015-destructuring' - ], + 'babel6-plugin-strip-heimdall' + ].concat(baseTransformPlugins) }).code; fs.writeFileSync('./dist/loader/loader.instrument.js', instrumented); +fs.writeFileSync('./dist/loader/loader.debug.js', debug); fs.writeFileSync('./dist/loader/loader.js', stripped); - diff --git a/lib/loader/loader.js b/lib/loader/loader.js index f433da5..6f76797 100644 --- a/lib/loader/loader.js +++ b/lib/loader/loader.js @@ -1,3 +1,5 @@ +import { DEBUG } from 'env-flags'; + var loader, define, requireModule, require, requirejs; (function(global) { @@ -112,12 +114,45 @@ var loader, define, requireModule, require, requirejs; } + if (DEBUG) { + var constructMakeDefaultExportDeprecationMessage = function(id) { + return 'The `' + id + '` module does not define a default export, but loader.js ' + + 'generated one anyway. This behavior is deprecated and will be removed in v5.0.0. ' + + 'To opt out of this behavior, set `loader.makeDefaultExport = false`.'; + }; + + var makeDefaultExportDeprecationOptions = { + until: '5.0.0', + id: 'loaderjs.makeDefaultExport' + }; + + loader.deprecationLogger = function() { + console.warn.apply(console, arguments); + }; + } + Module.prototype.makeDefaultExport = function() { var exports = this.module.exports; if (exports !== null && (typeof exports === 'object' || typeof exports === 'function') && exports['default'] === undefined && Object.isExtensible(exports)) { - exports['default'] = exports; + + if (DEBUG) { + var id = this.id; + + Object.defineProperty(exports, 'default', { + get: function () { + loader.deprecationLogger( + constructMakeDefaultExportDeprecationMessage(id), + false, + makeDefaultExportDeprecationOptions + ); + return exports; + } + }); + } else { + exports['default'] = exports; + } } }; diff --git a/package.json b/package.json index 903a80f..c2550c2 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,7 @@ "devDependencies": { "ara": "0.0.3", "babel-core": "^6.25.0", + "babel-plugin-debug-macros": "^0.1.10", "babel-plugin-transform-es2015-destructuring": "^6.23.0", "babel6-plugin-strip-heimdall": "^6.0.1", "heimdalljs": "^0.3.2", diff --git a/tests/all.js b/tests/all.js index f8ca441..fa7128b 100644 --- a/tests/all.js +++ b/tests/all.js @@ -60,6 +60,7 @@ test('has api', function() { strictEqual(define.amd, undefined); equal(typeof requirejs, 'function'); equal(typeof requireModule, 'function'); + equal(typeof loader.deprecationLogger, 'function'); }); test('no conflict mode', function() { @@ -434,6 +435,12 @@ test('assigns default when makeDefaultExport option enabled', function() { test('doesn\'t assign default when makeDefaultExport option is disabled', function() { var _loaderMakeDefaultExport = loader.makeDefaultExport; loader.makeDefaultExport = false; + + var deprecationArguments; + loader.deprecationLogger = function() { + deprecationArguments = [].slice.call(arguments); + }; + var theObject = {}; define('foo', ['require', 'exports', 'module'], function() { return theObject; @@ -456,6 +463,8 @@ test('doesn\'t assign default when makeDefaultExport option is disabled', functi pendingQueueLength: 1 }); + strictEqual(deprecationArguments, undefined); + // clean up loader.makeDefaultExport = _loaderMakeDefaultExport; }); @@ -724,6 +733,11 @@ test('if factory returns a value it is used as export', function() { }); test('if a module has no default property assume the return is the default', function() { + var deprecationArguments; + loader.deprecationLogger = function() { + deprecationArguments = [].slice.call(arguments); + }; + define('foo', [], function() { return { bar: 'bar' @@ -748,10 +762,21 @@ test('if a module has no default property assume the return is the default', fun }); equal(foo.bar, 'bar'); + + deepEqual(deprecationArguments, [ + 'The `foo` module does not define a default export, but loader.js generated one anyway. This behavior is deprecated and will be removed in v5.0.0. To opt out of this behavior, set `loader.makeDefaultExport = false`.', + false, + { id: 'loaderjs.makeDefaultExport', until: '5.0.0' } + ]); }); test('if a CJS style module has no default export assume module.exports is the default', function() { + var deprecationArguments; + loader.deprecationLogger = function() { + deprecationArguments = [].slice.call(arguments); + }; + define('Foo', ['require', 'exports', 'module'], function(require, exports, module) { module.exports = function Foo() { this.bar = 'bar'; @@ -776,10 +801,21 @@ test('if a CJS style module has no default export assume module.exports is the d resolveRelative: 0, pendingQueueLength: 1 }); + + deepEqual(deprecationArguments, [ + 'The `Foo` module does not define a default export, but loader.js generated one anyway. This behavior is deprecated and will be removed in v5.0.0. To opt out of this behavior, set `loader.makeDefaultExport = false`.', + false, + { id: 'loaderjs.makeDefaultExport', until: '5.0.0' } + ]); }); test('if a module has no default property assume its export is default (function)', function() { + var deprecationArguments; + loader.deprecationLogger = function() { + deprecationArguments = [].slice.call(arguments); + }; + var theFunction = function theFunction() {}; define('foo', ['require', 'exports', 'module'], function() { return theFunction; @@ -802,9 +838,20 @@ test('if a module has no default property assume its export is default (function resolveRelative: 0, pendingQueueLength: 1 }); + + deepEqual(deprecationArguments, [ + 'The `foo` module does not define a default export, but loader.js generated one anyway. This behavior is deprecated and will be removed in v5.0.0. To opt out of this behavior, set `loader.makeDefaultExport = false`.', + false, + { id: 'loaderjs.makeDefaultExport', until: '5.0.0' } + ]); }); test('if a module has no default property assume its export is default (object)', function() { + var deprecationArguments; + loader.deprecationLogger = function() { + deprecationArguments = [].slice.call(arguments); + }; + var theObject = {}; define('foo', ['require', 'exports', 'module'], function() { return theObject; @@ -827,9 +874,20 @@ test('if a module has no default property assume its export is default (object)' resolveRelative: 0, pendingQueueLength: 1 }); + + deepEqual(deprecationArguments, [ + 'The `foo` module does not define a default export, but loader.js generated one anyway. This behavior is deprecated and will be removed in v5.0.0. To opt out of this behavior, set `loader.makeDefaultExport = false`.', + false, + { id: 'loaderjs.makeDefaultExport', until: '5.0.0' } + ]); }); test('does not add default if export is frozen', function() { + var deprecationArguments; + loader.deprecationLogger = function() { + deprecationArguments = [].slice.call(arguments); + }; + var theObject = Object.freeze({}); define('foo', ['require', 'exports', 'module'], function() { return theObject; @@ -852,9 +910,16 @@ test('does not add default if export is frozen', function() { resolveRelative: 0, pendingQueueLength: 1 }); + + strictEqual(deprecationArguments, undefined); }); test('does not add default if export is sealed', function() { + var deprecationArguments; + loader.deprecationLogger = function() { + deprecationArguments = [].slice.call(arguments); + }; + var theObject = Object.seal({ derp: {} }); define('foo', ['require', 'exports', 'module'], function() { return theObject; @@ -877,6 +942,8 @@ test('does not add default if export is sealed', function() { resolveRelative: 0, pendingQueueLength: 1 }); + + strictEqual(deprecationArguments, undefined); }); test('has good error message for missing module', function() { diff --git a/yarn.lock b/yarn.lock index a53fb62..85bc6e0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -230,6 +230,12 @@ babel-plugin-dead-code-elimination@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/babel-plugin-dead-code-elimination/-/babel-plugin-dead-code-elimination-1.0.2.tgz#5f7c451274dcd7cccdbfbb3e0b85dd28121f0f65" +babel-plugin-debug-macros@^0.1.10: + version "0.1.10" + resolved "https://registry.yarnpkg.com/babel-plugin-debug-macros/-/babel-plugin-debug-macros-0.1.10.tgz#dd077ad6e1cc0a8f9bbc6405c561392ebfc9a01c" + dependencies: + semver "^5.3.0" + babel-plugin-eval@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/babel-plugin-eval/-/babel-plugin-eval-1.0.1.tgz#a2faed25ce6be69ade4bfec263f70169195950da" @@ -1962,7 +1968,7 @@ rsvp@~3.2.1: version "3.2.1" resolved "https://registry.yarnpkg.com/rsvp/-/rsvp-3.2.1.tgz#07cb4a5df25add9e826ebc67dcc9fd89db27d84a" -semver@^5.1.0: +semver@^5.1.0, semver@^5.3.0: version "5.3.0" resolved "https://registry.yarnpkg.com/semver/-/semver-5.3.0.tgz#9b2ce5d3de02d17c6012ad326aa6b4d0cf54f94f" From fc83edb30cbfe603577f47087ae0a5bba81a94d3 Mon Sep 17 00:00:00 2001 From: Lenny Burdette Date: Fri, 23 Jun 2017 17:14:36 -0700 Subject: [PATCH 2/2] feat(ember-cli): load debug build in Ember CLI dev/test builds This change adds a `loader.debug` option to Ember CLI builds. It defaults to true for development and test builds. When true, it prepends loader.debug.js instead of loader.js to the application's vendor.js. --- index.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index d701d1b..3128be0 100644 --- a/index.js +++ b/index.js @@ -10,13 +10,19 @@ module.exports = { this.treePaths['vendor'] = 'dist'; }, - included: function() { + included: function(app, parentAddon) { + var isProduction = process.env.EMBER_ENV === 'production'; + if (false /* hotfix */&& shouldUseInstrumentedBuild()) { - this.app.import('vendor/loader/loader.instrument.js', { + app.import('vendor/loader/loader.instrument.js', { + prepend: true + }); + } else if (!isProduction) { + app.import('vendor/loader/loader.debug.js', { prepend: true - }) + }); } else { - this.app.import('vendor/loader/loader.js', { + app.import('vendor/loader/loader.js', { prepend: true }); }