bug #35 Fixing sourceMap bug and adding "lazy" & "export" loader options (weaverryan)

This PR was merged into the main branch.

Discussion
----------

Fixing sourceMap bug and adding "lazy" & "export" loader options

Fixes #33 (except for lazy controllers... that would be more complex).

Hi!

This PR contains 2 parts:

1) The sourcemap problem described in #33 is fixed. Thanks to @Kocal for that - he had all the right details... they just needed to be in a slightly different place. We don't need this sourcemap trick in the actual `loader.js` because that is loading a `controllers.json` file - were not trying to map eventual errors to that file.

2) This adds the ability to add a `?lazy=true` when using the `lazy-controller-loader`, allowing you to make 3rd party controllers lazy (though the syntax isn't super attractive). See the updated README.

I also tested this on a real project.

Cheers!

Commits
-------

c3c0111 Fixing sourceMap bug and adding ability to specify laziness and export names as loader query

Author: tgalopin

README.md

@@ -230,6 +230,43 @@ export default class extends Controller {
 }
 ```
 
+### Advanced Lazy Controllers
+
+Sometimes you may want to use a third-party controller and make it lazy.
+Unfortunately, you can't edit that controller's source code to add
+the `/* stimulusFetch: 'lazy' */`.
+
+To handle this, you can use the `lazy-controller-loader` with some
+custom query options.
+
+```js
+// assets/bootstrap.js
+
+import { startStimulusApp } from '@symfony/stimulus-bridge';
+
+// example from https://stimulus-components.netlify.app/docs/components/stimulus-clipboard/
+// normal, non-lazy import
+//import Clipboard from 'stimulus-clipboard';
+// lazy import
+import Clipboard from '@symfony/stimulus-bridge/lazy-controller-loader?lazy=true!stimulus-clipboard';
+
+// example from https://github.com/afcapel/stimulus-autocomplete
+// normal, non-lazy import
+//import { Autocomplete } from 'stimulus-autocomplete';
+// lazy import - it includes export=Autocomplete to handle the named export
+import { Autocomplete } from '@symfony/stimulus-bridge/lazy-controller-loader?lazy=true&export=Autocomplete!stimulus-autocomplete';
+
+const app = startStimulusApp(require.context(
+    // your existing code to load from controllers/
+));
+
+// the normal way to manually register controllers
+application.register('clipboard', Clipboard)
+application.register('autocomplete', Autocomplete)
+
+export { app };
+```
+
 ## Run tests
 
 ```sh

dist/webpack/generate-lazy-controller.js

@@ -11,9 +11,11 @@
  *
  * @param {string} controllerPath The importable path to the controller
  * @param {Number} indentationSpaces Amount each line should be indented
+ * @param {string} exportName The name of the module that's exported from the controller
  */
 
 module.exports = function generateLazyController(controllerPath, indentationSpaces) {
+  var exportName = arguments.length > 2 && arguments[2] !== undefined ? arguments[2] : 'default';
   var spaces = ' '.repeat(indentationSpaces);
-  return "".concat(spaces, "(function() {\n").concat(spaces, "    function LazyController(context) {\n").concat(spaces, "        this.__stimulusLazyController = true;\n").concat(spaces, "        Controller.call(this, context);\n").concat(spaces, "    }\n").concat(spaces, "    LazyController.prototype = Object.create(Controller && Controller.prototype, {\n").concat(spaces, "        constructor: { value: LazyController, writable: true, configurable: true }\n").concat(spaces, "    });\n").concat(spaces, "    Object.setPrototypeOf(LazyController, Controller);\n").concat(spaces, "    LazyController.prototype.initialize = function() {\n").concat(spaces, "        var _this = this;\n").concat(spaces, "        if (this.application.controllers.find(function(controller) {\n").concat(spaces, "            return controller.identifier === _this.identifier && controller.__stimulusLazyController;\n").concat(spaces, "        })) {\n").concat(spaces, "            return;\n").concat(spaces, "        }\n").concat(spaces, "        import('").concat(controllerPath.replace(/\\/g, '\\\\'), "').then(function(controller) {\n").concat(spaces, "            _this.application.register(_this.identifier, controller.default);\n").concat(spaces, "        });\n").concat(spaces, "    }\n").concat(spaces, "    return LazyController;\n").concat(spaces, "})()");
+  return "".concat(spaces, "(function() {\n").concat(spaces, "    function LazyController(context) {\n").concat(spaces, "        this.__stimulusLazyController = true;\n").concat(spaces, "        Controller.call(this, context);\n").concat(spaces, "    }\n").concat(spaces, "    LazyController.prototype = Object.create(Controller && Controller.prototype, {\n").concat(spaces, "        constructor: { value: LazyController, writable: true, configurable: true }\n").concat(spaces, "    });\n").concat(spaces, "    Object.setPrototypeOf(LazyController, Controller);\n").concat(spaces, "    LazyController.prototype.initialize = function() {\n").concat(spaces, "        var _this = this;\n").concat(spaces, "        if (this.application.controllers.find(function(controller) {\n").concat(spaces, "            return controller.identifier === _this.identifier && controller.__stimulusLazyController;\n").concat(spaces, "        })) {\n").concat(spaces, "            return;\n").concat(spaces, "        }\n").concat(spaces, "        import('").concat(controllerPath.replace(/\\/g, '\\\\'), "').then(function(controller) {\n").concat(spaces, "            _this.application.register(_this.identifier, controller.").concat(exportName, ");\n").concat(spaces, "        });\n").concat(spaces, "    }\n").concat(spaces, "    return LazyController;\n").concat(spaces, "})()");
 };

dist/webpack/lazy-controller-loader.js

@@ -17,6 +17,24 @@ function _arrayLikeToArray(arr, len) { if (len == null || len > arr.length) len
 var generateLazyController = require('./generate-lazy-controller');
 
 var getStimulusCommentOptions = require('../util/get-stimulus-comment-options');
+
+var _require = require('loader-utils'),
+    getOptions = _require.getOptions;
+
+var _require2 = require('schema-utils'),
+    validate = _require2.validate;
+
+var schema = {
+  type: 'object',
+  properties: {
+    lazy: {
+      type: 'boolean'
+    },
+    "export": {
+      type: 'string'
+    }
+  }
+};
 /**
  * Loader that can make a Stimulus controller lazy.
  *
@@ -27,11 +45,12 @@ var getStimulusCommentOptions = require('../util/get-stimulus-comment-options');
  * element appears.
  *
  * @param {string} source of a module that exports a Stimulus controller
+ * @param {string} sourceMap the current source map string
+ *
  * @return {string}
  */
 
-
-module.exports = function (source) {
+module.exports = function (source, sourceMap) {
   var _getStimulusCommentOp = getStimulusCommentOptions(source),
       options = _getStimulusCommentOp.options,
       errors = _getStimulusCommentOp.errors;
@@ -56,11 +75,22 @@ module.exports = function (source) {
     this.emitError(new Error("Invalid value \"".concat(stimulusFetch, "\" found for \"stimulusFetch\". Allowed values are \"lazy\" or \"eager\"")));
   }
 
-  var isLazy = stimulusFetch === 'lazy';
+  var loaderOptions = getOptions(this);
+  validate(schema, loaderOptions, {
+    name: '@symfony/stimulus-bridge/lazy-controller-loader',
+    baseDataPath: 'options'
+  }); // the ?lazy= loader option takes priority over the comment
+
+  var isLazy = typeof loaderOptions.lazy !== 'undefined' ? loaderOptions.lazy : stimulusFetch === 'lazy';
 
   if (!isLazy) {
-    return source;
+    return this.callback(null, source, sourceMap);
   }
 
-  return "import { Controller } from 'stimulus';\nexport default ".concat(generateLazyController(this.resource, 0));
+  var exportName = typeof loaderOptions["export"] !== 'undefined' ? loaderOptions["export"] : 'default';
+  var finalSource = "import { Controller } from 'stimulus';\nconst controller = ".concat(generateLazyController(this.resource, 0, exportName), ";\nexport { controller as ").concat(exportName, " };"); // The source Map cannot be passed when lazy, as the sourceMap won't
+  // map up to the new source. In theory, this is fixable, but I'm
+  // not entirely sure how.
+
+  this.callback(null, finalSource);
 };

package.json

@@ -16,7 +16,9 @@
         "stimulus": "^2.0"
     },
     "dependencies": {
-        "acorn": "^8.0.5"
+        "acorn": "^8.0.5",
+        "loader-utils": "^2.0.0",
+        "schema-utils": "^3.0.0"
     },
     "devDependencies": {
         "@babel/cli": "^7.12.1",

src/webpack/generate-lazy-controller.js

@@ -13,8 +13,9 @@
  *
  * @param {string} controllerPath The importable path to the controller
  * @param {Number} indentationSpaces Amount each line should be indented
+ * @param {string} exportName The name of the module that's exported from the controller
  */
-module.exports = function generateLazyController(controllerPath, indentationSpaces) {
+module.exports = function generateLazyController(controllerPath, indentationSpaces, exportName = 'default') {
     const spaces = ' '.repeat(indentationSpaces);
 
     return `${spaces}(function() {
@@ -34,7 +35,7 @@ ${spaces}        })) {
 ${spaces}            return;
 ${spaces}        }
 ${spaces}        import('${controllerPath.replace(/\\/g, '\\\\')}').then(function(controller) {
-${spaces}            _this.application.register(_this.identifier, controller.default);
+${spaces}            _this.application.register(_this.identifier, controller.${exportName});
 ${spaces}        });
 ${spaces}    }
 ${spaces}    return LazyController;

src/webpack/lazy-controller-loader.js

@@ -11,6 +11,20 @@
 
 const generateLazyController = require('./generate-lazy-controller');
 const getStimulusCommentOptions = require('../util/get-stimulus-comment-options');
+const { getOptions } = require('loader-utils');
+const { validate } = require('schema-utils');
+
+const schema = {
+    type: 'object',
+    properties: {
+        lazy: {
+            type: 'boolean',
+        },
+        export: {
+            type: 'string',
+        },
+    },
+};
 
 /**
  * Loader that can make a Stimulus controller lazy.
@@ -22,9 +36,11 @@ const getStimulusCommentOptions = require('../util/get-stimulus-comment-options'
  * element appears.
  *
  * @param {string} source of a module that exports a Stimulus controller
+ * @param {string} sourceMap the current source map string
+ *
  * @return {string}
  */
-module.exports = function (source) {
+module.exports = function (source, sourceMap) {
     const { options, errors } = getStimulusCommentOptions(source);
 
     for (const error of errors) {
@@ -41,12 +57,29 @@ module.exports = function (source) {
             )
         );
     }
-    const isLazy = stimulusFetch === 'lazy';
+
+    const loaderOptions = getOptions(this);
+
+    validate(schema, loaderOptions, {
+        name: '@symfony/stimulus-bridge/lazy-controller-loader',
+        baseDataPath: 'options',
+    });
+
+    // the ?lazy= loader option takes priority over the comment
+    const isLazy = typeof loaderOptions.lazy !== 'undefined' ? loaderOptions.lazy : stimulusFetch === 'lazy';
 
     if (!isLazy) {
-        return source;
+        return this.callback(null, source, sourceMap);
     }
 
-    return `import { Controller } from 'stimulus';
-export default ${generateLazyController(this.resource, 0)}`;
+    const exportName = typeof loaderOptions.export !== 'undefined' ? loaderOptions.export : 'default';
+
+    const finalSource = `import { Controller } from 'stimulus';
+const controller = ${generateLazyController(this.resource, 0, exportName)};
+export { controller as ${exportName} };`;
+
+    // The source Map cannot be passed when lazy, as the sourceMap won't
+    // map up to the new source. In theory, this is fixable, but I'm
+    // not entirely sure how.
+    this.callback(null, finalSource);
 };

test/webpack/generate-lazy-controller.test.js

@@ -30,6 +30,7 @@ describe('generateLazyControllerModule', () => {
             const lazyControllerClass = eval(`${controllerCode}`);
             // if all goes correctly, the prototype should have a Controller key
             expect(Object.getPrototypeOf(lazyControllerClass)).toHaveProperty('Controller');
+            expect(controllerCode).toContain('_this.application.register(_this.identifier, controller.default)');
         });
 
         it('must return a functional ES5 class on Windows', () => {
@@ -52,5 +53,21 @@ describe('generateLazyControllerModule', () => {
              */
             expect(controllerCode).toContain(`import('C:\\\\\\\\path\\\\to\\\\file.js')`);
         });
+
+        it('must use the correct, named export', () => {
+            const controllerCode =
+                "const Controller = require('stimulus');\n" +
+                // this, for some reason, is undefined in a test but populated in a real situation
+                // this avoid an explosion since it is undefined here
+                'Controller.prototype = {};\n' +
+                generateLazyController('@symfony/some-module/dist/controller.js', 0, 'CustomController');
+            const result = babelParser.parse(controllerCode, {
+                sourceType: 'module',
+            });
+            expect(babelTypes.isNode(result)).toBeTruthy();
+            expect(controllerCode).toContain(
+                '_this.application.register(_this.identifier, controller.CustomController)'
+            );
+        });
     });
 });

test/webpack/lazy-controller-loader.test.js

@@ -11,40 +11,79 @@
 
 const lazyControllerLoader = require('../../dist/webpack/lazy-controller-loader');
 
-function callLoader(src, errors = []) {
+function callLoader(src, startingSourceMap = '', query = '') {
     const loaderThis = {
+        emittedErrors: [],
+        executedCallback: null,
+
         resource: './some-resource',
+        query,
         emitError(error) {
-            errors.push(error);
+            this.emittedErrors.push(error);
+        },
+        callback(error, content, sourceMap) {
+            this.executedCallback = { error, content, sourceMap };
         },
     };
 
-    return lazyControllerLoader.call(loaderThis, src);
+    lazyControllerLoader.call(loaderThis, src, startingSourceMap);
+
+    return {
+        content: loaderThis.executedCallback.content,
+        errors: loaderThis.emittedErrors,
+        sourceMap: loaderThis.executedCallback.sourceMap,
+        callbackErrors: loaderThis.executedCallback.errors,
+    };
 }
 
 describe('lazyControllerLoader', () => {
     it('does nothing with a non-lazy controller', () => {
         const src = 'export default class extends Controller {}';
-        expect(callLoader(src)).toEqual(src);
+        expect(callLoader(src).content).toEqual(src);
+        expect(callLoader(src, 'source_map_contents').sourceMap).toEqual('source_map_contents');
+        expect(callLoader(src).errors).toHaveLength(0);
     });
 
     it('it exports a lazy controller', () => {
         const src = "/* stimulusFetch: 'lazy' */ export default class extends Controller {}";
         // look for a little bit of the lazy controller code
-        expect(callLoader(src)).toContain('application.register(');
+        expect(callLoader(src).content).toContain('function LazyController');
+        // unfortunately, we cannot pass along sourceMap info since we changed the source
+        expect(callLoader(src, 'source_map_contents').sourceMap).toBeUndefined();
+        expect(callLoader(src).errors).toHaveLength(0);
     });
 
     it('it emits an error on a syntax problem', () => {
         const src = '/* stimulusFetch: "lazy */ export default class extends Controller {}';
-        const errors = [];
-        callLoader(src, errors);
-        expect(errors).toHaveLength(1);
+        expect(callLoader(src).errors).toHaveLength(1);
     });
 
     it('it emits an error on an invalid value', () => {
         const src = '/* stimulusFetch: "lazy-once" */ export default class extends Controller {}';
-        const errors = [];
-        callLoader(src, errors);
-        expect(errors).toHaveLength(1);
+        expect(callLoader(src).errors).toHaveLength(1);
+    });
+
+    it('it reads ?lazy option', () => {
+        const src = 'export default class extends Controller {}';
+        const results = callLoader(src, '', '?lazy=true');
+        expect(results.content).toContain('function LazyController');
+        expect(results.errors).toHaveLength(0);
+    });
+
+    it('it reads ?lazy and it wins over comments', () => {
+        const src = "/* stimulusFetch: 'eager' */ export default class extends Controller {}";
+        const results = callLoader(src, '', '?lazy=true');
+        expect(results.content).toContain('function LazyController');
+        expect(results.errors).toHaveLength(0);
+    });
+
+    it('it reads ?export for non-default exports', () => {
+        const src = 'const MyController = class extends Controller {}; export { MyController };';
+        const results = callLoader(src, '', '?lazy=true&export=MyController');
+        // check that the results are lazy
+        expect(results.content).toContain('function LazyController');
+        // check named export
+        expect(results.content).toContain('export { controller as MyController };');
+        expect(results.errors).toHaveLength(0);
     });
 });