Skip to content

"fetch: lazy": don't download until the controller is present #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@
Before: `@symfony/ux-dropzone/dropzone`
After: `symfony--ux-dropzone--dropzone`

* Support for "lazy controllers" was added. By setting the `fetch`
in `controllers.json` to `lazy`, your controller will not
be downloaded until the controller element first appears on the page.

* The `webpackMode` option in `controllers.json` was deprecated. Use
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should open a PR on UX and Flex to change this there. Do you have time to do it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already did! :) symfony/ux#53

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But flex - I hadn’t thought about that piece - I’ll get that done

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flex PR: symfony/flex#735

the new `fetch` option instead.

## 1.1.0

* Support for Stimulus 1 dropped and support for Stimulus 2 added - #4.
Expand Down
38 changes: 38 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,44 @@ If you get the error:

Be sure to upgrade to `@symfony/webpack-encore` version 1.0.0 or higher.

## The controllers.json File

The bridge works by reading a `controllers.json` file that is automatically
updated by Symfony Flex whenever you download a UX-powered package. For
example, after running `composer require symfony/ux-dropzone`, it will
look like this:

```json
{
"controllers": {
"@symfony/ux-dropzone": {
"dropzone": {
"enabled": true,
"fetch": "eager",
"autoimport": {
"@symfony/ux-dropzone/src/style.css": true
}
}
}
},
"entrypoints": []
}
```

Each item under `controllers` will cause a Stimulus controller to be
registered with a specific name - in this case the controller would
be called `symfony--ux-dropzone--dropzone` (the `/` becomes `--`).

By default, the new controller will always be included in your
JavaScript package. You can control that with the `fetch` option,
ordered from least to most lazy:

* `fetch: 'eager'`: controller & dependencies are included in the JavaScript
that's downloaded when the page is loaded.
* `fetch: 'lazy'`: controller & dependencies are isolated into a
separate file and only downloaded asynchronously if (and when) the `data-controller`
HTML appears on the page.

## Run tests

```sh
Expand Down
13 changes: 4 additions & 9 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,14 @@ function startStimulusApp(context) {
application.load((0, _webpackHelpers.definitionsFromContext)(context));
}

var _loop = function _loop(_controllerName) {
if (!_controllers["default"].hasOwnProperty(_controllerName)) {
controllerName = _controllerName;
var _loop = function _loop(controllerName) {
if (!_controllers["default"].hasOwnProperty(controllerName)) {
return "continue";
}

_controllers["default"][_controllerName].then(function (module) {
// Normalize the controller name: remove the initial @ and use Stimulus format
_controllerName = _controllerName.substr(1).replace(/_/g, '-').replace(/\//g, '--');
application.register(_controllerName, module["default"]);
_controllers["default"][controllerName].then(function (module) {
application.register(controllerName, module["default"]);
});

controllerName = _controllerName;
};

for (var controllerName in _controllers["default"]) {
Expand Down
41 changes: 37 additions & 4 deletions dist/webpack/create-controllers-module.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@
*/
'use strict';

var generateLazyController = require('./generate-lazy-controller');

module.exports = function createControllersModule(config) {
var controllerContents = 'export default {';
var autoImportContents = '';
var hasLazyControllers = false;
var deprecations = [];

if ('undefined' !== typeof config['placeholder']) {
throw new Error('Your controllers.json file was not found. Be sure to add a Webpack alias from "@symfony/stimulus-bridge/controllers.json" to *your* controllers.json file.');
Expand All @@ -24,7 +28,9 @@ module.exports = function createControllersModule(config) {
var packageConfig = require(packageName + '/package.json');

for (var controllerName in config.controllers[packageName]) {
var controllerReference = packageName + '/' + controllerName; // Find package config for the controller
var controllerReference = packageName + '/' + controllerName; // Normalize the controller name: remove the initial @ and use Stimulus format

var controllerNormalizedName = controllerReference.substr(1).replace(/_/g, '-').replace(/\//g, '--'); // Find package config for the controller

if ('undefined' === typeof packageConfig.symfony.controllers[controllerName]) {
throw new Error('Controller "' + controllerReference + '" does not exist in the package and cannot be compiled.');
Expand All @@ -38,8 +44,28 @@ module.exports = function createControllersModule(config) {
}

var controllerMain = packageName + '/' + controllerPackageConfig.main;
var webpackMode = controllerUserConfig.webpackMode;
controllerContents += "\n '" + controllerReference + '\': import(/* webpackMode: "' + webpackMode + '" */ \'' + controllerMain + "'),";
var fetchMode = 'eager';

if ('undefined' !== typeof controllerUserConfig.webpackMode) {
deprecations.push('The "webpackMode" config key is deprecated in controllers.json. Use "fetch" instead, set to either "eager" or "lazy".');
}

if ('undefined' !== typeof controllerUserConfig.fetch) {
if (!['eager', 'lazy'].includes(controllerUserConfig.fetch)) {
throw new Error("Invalid \"fetch\" value \"".concat(controllerUserConfig.fetch, "\" in controllers.json. Expected \"eager\" or \"lazy\"."));
}

fetchMode = controllerUserConfig.fetch;
}

var moduleValueContents = "import(/* webpackMode: \"eager\" */ '".concat(controllerMain, "')");

if (fetchMode === 'lazy') {
hasLazyControllers = true;
moduleValueContents = "\nnew Promise((resolve, reject) => resolve({ default:\n".concat(generateLazyController(controllerMain, 6), "\n }))\n ").trim();
}

controllerContents += "\n '".concat(controllerNormalizedName, "': ").concat(moduleValueContents, ",");

for (var autoimport in controllerUserConfig.autoimport || []) {
if (controllerUserConfig.autoimport[autoimport]) {
Expand All @@ -49,5 +75,12 @@ module.exports = function createControllersModule(config) {
}
}

return "".concat(autoImportContents).concat(controllerContents, "\n};");
if (hasLazyControllers) {
controllerContents = "import { Controller } from 'stimulus';\n".concat(controllerContents);
}

return {
finalSource: "".concat(autoImportContents).concat(controllerContents, "\n};"),
deprecations: deprecations
};
};
19 changes: 19 additions & 0 deletions dist/webpack/generate-lazy-controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
'use strict';
/**
*
* @param {string} controllerPath The importable path to the controller
* @param {Number} indentationSpaces Amount each line should be indented
*/

module.exports = function generateLazyController(controllerPath, indentationSpaces) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we have dist files committed in this repo ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should clean this up. Some dist files are needed because they're read at runtime in the browser - specifically the src/index.js file (which the user imports directly from their code and uses. But other things - like these loaders - are only executed at build time and do not need a dist.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even for the necessary ones, I don't think they need to be committed, as the npm ecosystem relies on a separate publishing step and so the VCS content and the package content can be different.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right about that too. None of them should be committed. However, we would need to make sure there was a more sophisticated build+publish functionality. The dist files are also committed in symfony/ux for the same reason: ease of distribution (because there is no automated build/release procedure).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

symfony/ux packages are not distributed through npm but through composer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm already has a built-in automation: it will run the prepublishOnly script before building the archive for publication.
you only need to update the package.json to make that prepublishOnly script run npm run build and that's automated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repo could definitely not include the dist files at all indeed. I added them without too much thoughts after having added them in symfony/ux.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's fix that in another PR

var spaces = ' '.repeat(indentationSpaces);
return "".concat(spaces, "(function() {\n").concat(spaces, " function LazyController(context) {\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, " import('").concat(controllerPath, "').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, "})()");
};
33 changes: 31 additions & 2 deletions dist/webpack/loader.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,32 @@
"use strict";
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
'use strict';

var LoaderDependency = require('webpack/lib/dependencies/LoaderDependency');

var createControllersModule = require('./create-controllers-module');
/**
* Loader that processes the controllers.json file.
*
* This reads the controllers key and returns an object
* where the keys are each controller name and the value
* is the module name (or inline class for lazy controllers)
* for that controller.
*
* @param {string} source controllers.json source
* @return {string}
*/


module.exports = function (source) {
var _this = this;

var logger = this.getLogger('stimulus-bridge-loader');
/*
* The following code prevents the normal JSON loader from
Expand All @@ -24,5 +46,12 @@ module.exports = function (source) {
this._module.parser = factory.getParser(requiredType);
/* End workaround */

return createControllersModule(JSON.parse(source));
var _createControllersMod = createControllersModule(JSON.parse(source)),
finalSource = _createControllersMod.finalSource,
deprecations = _createControllersMod.deprecations;

deprecations.forEach(function (message) {
_this.emitWarning(new Error(message));
});
return finalSource;
};
3 changes: 0 additions & 3 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ export function startStimulusApp(context) {
}

symfonyControllers[controllerName].then((module) => {
// Normalize the controller name: remove the initial @ and use Stimulus format
controllerName = controllerName.substr(1).replace(/_/g, '-').replace(/\//g, '--');

application.register(controllerName, module.default);
});
}
Expand Down
52 changes: 42 additions & 10 deletions src/webpack/create-controllers-module.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@

'use strict';

const generateLazyController = require('./generate-lazy-controller');

module.exports = function createControllersModule(config) {
let controllerContents = 'export default {';
let autoImportContents = '';
let hasLazyControllers = false;
const deprecations = [];

if ('undefined' !== typeof config['placeholder']) {
throw new Error(
Expand All @@ -28,6 +32,8 @@ module.exports = function createControllersModule(config) {

for (let controllerName in config.controllers[packageName]) {
const controllerReference = packageName + '/' + controllerName;
// Normalize the controller name: remove the initial @ and use Stimulus format
const controllerNormalizedName = controllerReference.substr(1).replace(/_/g, '-').replace(/\//g, '--');

// Find package config for the controller
if ('undefined' === typeof packageConfig.symfony.controllers[controllerName]) {
Expand All @@ -44,16 +50,35 @@ module.exports = function createControllersModule(config) {
}

const controllerMain = packageName + '/' + controllerPackageConfig.main;
const webpackMode = controllerUserConfig.webpackMode;
let fetchMode = 'eager';

if ('undefined' !== typeof controllerUserConfig.webpackMode) {
deprecations.push(
'The "webpackMode" config key is deprecated in controllers.json. Use "fetch" instead, set to either "eager" or "lazy".'
);
}

if ('undefined' !== typeof controllerUserConfig.fetch) {
if (!['eager', 'lazy'].includes(controllerUserConfig.fetch)) {
throw new Error(
`Invalid "fetch" value "${controllerUserConfig.fetch}" in controllers.json. Expected "eager" or "lazy".`
);
}

controllerContents +=
"\n '" +
controllerReference +
'\': import(/* webpackMode: "' +
webpackMode +
'" */ \'' +
controllerMain +
"'),";
fetchMode = controllerUserConfig.fetch;
}

let moduleValueContents = `import(/* webpackMode: "eager" */ '${controllerMain}')`;
if (fetchMode === 'lazy') {
hasLazyControllers = true;
moduleValueContents = `
new Promise((resolve, reject) => resolve({ default:
${generateLazyController(controllerMain, 6)}
}))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this runs after babel, we need to rewrite this into "old" JavaScript to support all browsers. I have the code locally, just need to put it in here :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or maybe we can make this run before babel

Copy link
Member Author

@weaverryan weaverryan Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be nice... I just could not find a way to do that :/. We effectively would need to change the "filename" of the output source to something that ends in .js and get Webpack to, sort of, "restart" the process so that the normal .js loader can match it. That may be possible, but I couldn't find away. And... really, we ship "dist" code normally that has been pre-compiled with Babel. This just a special case of doing the same (the strangeness being that I'll rewrite this to ES5 manually).

`.trim();
}

controllerContents += `\n '${controllerNormalizedName}': ${moduleValueContents},`;

for (let autoimport in controllerUserConfig.autoimport || []) {
if (controllerUserConfig.autoimport[autoimport]) {
Expand All @@ -63,5 +88,12 @@ module.exports = function createControllersModule(config) {
}
}

return `${autoImportContents}${controllerContents}\n};`;
if (hasLazyControllers) {
controllerContents = `import { Controller } from 'stimulus';\n${controllerContents}`;
}

return {
finalSource: `${autoImportContents}${controllerContents}\n};`,
deprecations,
};
};
36 changes: 36 additions & 0 deletions src/webpack/generate-lazy-controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

'use strict';

/**
*
* @param {string} controllerPath The importable path to the controller
* @param {Number} indentationSpaces Amount each line should be indented
*/
module.exports = function generateLazyController(controllerPath, indentationSpaces) {
const spaces = ' '.repeat(indentationSpaces);

return `${spaces}(function() {
${spaces} function LazyController(context) {
${spaces} Controller.call(this, context);
${spaces} }
${spaces} LazyController.prototype = Object.create(Controller && Controller.prototype, {
${spaces} constructor: { value: LazyController, writable: true, configurable: true }
${spaces} });
${spaces} Object.setPrototypeOf(LazyController, Controller);
${spaces} LazyController.prototype.initialize = function() {
${spaces} var _this = this;
${spaces} import('${controllerPath}').then(function(controller) {
${spaces} _this.application.register(_this.identifier, controller.default);
${spaces} });
${spaces} }
${spaces} return LazyController;
${spaces}})()`;
};
30 changes: 29 additions & 1 deletion src/webpack/loader.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,28 @@
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

'use strict';

const LoaderDependency = require('webpack/lib/dependencies/LoaderDependency');
const createControllersModule = require('./create-controllers-module');

/**
* Loader that processes the controllers.json file.
*
* This reads the controllers key and returns an object
* where the keys are each controller name and the value
* is the module name (or inline class for lazy controllers)
* for that controller.
*
* @param {string} source controllers.json source
* @return {string}
*/
module.exports = function (source) {
const logger = this.getLogger('stimulus-bridge-loader');

Expand All @@ -18,5 +40,11 @@ module.exports = function (source) {
this._module.parser = factory.getParser(requiredType);
/* End workaround */

return createControllersModule(JSON.parse(source));
const { finalSource, deprecations } = createControllersModule(JSON.parse(source));

deprecations.forEach((message) => {
this.emitWarning(new Error(message));
});

return finalSource;
};
Loading