Make config run async #1429

MadeByMike · 2020-09-27T06:14:16Z

This allows for acync config. Such as:

module.exports = async (eleventyConfig) => {
   const data = await something();
   eleventyConfig.doSomethingWith(data)
}

I know this looks like a HEFTY PR please ignore the number of files changed and look at the types of changes. Significan't changes to the way tests are structure and how templateConfig is handled but the changes to the core core are relatively small.

The main changes are to cmd.js and the main Eleventy class. A number of calls in main 11ty runner are now delayed until after the config has been initialised. Other significant changes include the way plugins are added. These are now also async so they are resisted and resolved when config is run.

Finally the way the global template config is managed has changed. Previously this could be imported from src/Config.js into anywhere and this would work since it is global. Now, since config is async we need to ensure that it is passed explicitly to each classes that uses it after it has been initialised.

Because of this there were also significant changes to tests to ensure that the config has been initialised before tests that depend on it.

There are a few todos including:

More tests of async config functionality
Tests to ensure the merging of plugins always occurs in the order added

Throwing this up here for feedback and to test the CI. Since 11ty doesn't ship a lock file I sometimes get differences with the way prism does highlighting.

I think the passing around of templateConfig could be improved by adding a base class that sets and manages these global properties. Tests could then override this property for individual classes. Happy to consider these type of suggestions or for future improvement to 11ty in general.

MadeByMike · 2020-09-27T06:57:48Z

Fixes: #614

…into async-configs

MadeByMike · 2020-09-28T02:43:50Z

I've been testing this in a real setting today and I hit this:

(node:8341) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 afterBuild listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit

I'm not sure if it is related to this change or not but it looks like then the config changes the beforeBuild and afterBuild listeners is added each time.

We should test this... might be happening with the existing prod build.

MadeByMike · 2020-09-28T02:53:46Z

I found a bug where changes to the .md files correctly trigger a rebuild but changes to .njk files trigger the watcher (and rebuild the file) but without the updated data.

I might need help tracking this one down... probably in mergeConfig ?

MadeByMike · 2020-10-06T00:17:51Z

Got it! The data is correct. I'd moved the initialization of the watcher until after the config was initialized because it needed the config in the constructor. However init is run on change so I ended up with a new browser-sync server instance every time files changed and thus... no reload.

Fixed by changing the watcher constructor.

MadeByMike · 2020-10-06T22:00:57Z

I've been testing this in a real setting today and I hit this:

(node:8341) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 afterBuild listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit

I think this was solved by ensuring the config init() was only called once

zachleat

Really sorry to do this but adding config arguments to constructors isn’t likely ideal for future maintenance—can you rework those? 😭

cmd.js

src/Eleventy.js

zachleat · 2020-10-16T15:57:27Z

src/Eleventy.js

    this.config.inputDir = this.inputDir;

    let formats = this.formatsOverride || this.config.templateFormats;
-    this.extensionMap = new EleventyExtensionMap(formats);
+    this.extensionMap = new EleventyExtensionMap(formats, config);


I’d super rather just use the setter style here rather than changing the constructor signature. this.extensionMap.config =

I can attempt this, but in some cases config is already used in the constructor and I'm not sure I know how to handle that.

I could really use an example I think.

zachleat · 2020-10-16T15:58:16Z

src/Eleventy.js


-    this.templateData = new TemplateData(this.inputDir);
+    this.templateData = new TemplateData(this.inputDir, config);


Same as the comment above but it looks like this one will be a little more annoying because there isn’t a setter in place already

zachleat · 2020-10-16T17:35:02Z

src/TemplateConfig.js

    let localConfig = {};
    let path = TemplatePath.join(
      TemplatePath.getWorkingDir(),
      localProjectConfigPath
    );
    debug(`Merging config with ${path}`);

+    // Note for Mike: I'm delaying the processing of plugins until here.


Did you have your think here 😅

Yes I did if by think you mean run enough scenarios in a production scale project until you hit the bugs.

What I did was move the merging of the config part to a new method: mergeEleventyConfig now I run this before and after processing the plugins. This means that the plugins get the initial merged data and that any data changes\added by plugins is remerged.

let merged = this.mergeEleventyConfig(localConfig); await eleventyConfig.applyPlugins(merged); merged = this.mergeEleventyConfig(localConfig);

For performance reasons I chose not to merge after each individual plugin, meaning plugins can't read data added or changed by other plugins, and order of plugins doesn't matter.

Will delete comment.

src/Eleventy.js

MadeByMike · 2020-10-18T19:27:25Z

src/Eleventy.js

    this.config.inputDir = this.inputDir;

    let formats = this.formatsOverride || this.config.templateFormats;
-    this.extensionMap = new EleventyExtensionMap(formats);
+    this.extensionMap = new EleventyExtensionMap(formats, config);


I can attempt this, but in some cases config is already used in the constructor and I'm not sure I know how to handle that.

MadeByMike · 2020-10-18T19:30:29Z

src/EleventyExtensionMap.js

-    return this.configOverride || require("./Config").getConfig();
+    return this.configOverride || this.config;


Another reason I am confused about using a getter\setter is that config is not the same thing as templateConfig need multiple getters\setters?

MadeByMike · 2020-10-18T19:30:55Z

src/EleventyExtensionMap.js

  }
  set config(cfg) {
    this.configOverride = cfg;
  }

  get engineManager() {
    if (!this._engineManager) {
-      this._engineManager = new TemplateEngineManager();
+      this._engineManager = new TemplateEngineManager(this.templateConfig);


Getter/Setter here too?

MadeByMike · 2020-10-18T19:33:12Z

src/Engines/Handlebars.js

@@ -2,8 +2,8 @@ const HandlebarsLib = require("handlebars");
 const TemplateEngine = require("./TemplateEngine");

 class Handlebars extends TemplateEngine {
-  constructor(name, includesDir) {
-    super(name, includesDir);
+  constructor(name, includesDir, templateConfig) {


Getter/Setter here too?

MadeByMike · 2020-10-18T19:46:47Z

src/TemplateConfig.js

    let localConfig = {};
    let path = TemplatePath.join(
      TemplatePath.getWorkingDir(),
      localProjectConfigPath
    );
    debug(`Merging config with ${path}`);

+    // Note for Mike: I'm delaying the processing of plugins until here.


Yes I did if by think you mean run enough scenarios in a production scale project until you hit the bugs.

What I did was move the merging of the config part to a new method: mergeEleventyConfig now I run this before and after processing the plugins. This means that the plugins get the initial merged data and that any data changes\added by plugins is remerged.

let merged = this.mergeEleventyConfig(localConfig); await eleventyConfig.applyPlugins(merged); merged = this.mergeEleventyConfig(localConfig);

For performance reasons I chose not to merge after each individual plugin, meaning plugins can't read data added or changed by other plugins, and order of plugins doesn't matter.

Will delete comment.

MadeByMike · 2020-10-18T19:50:43Z

src/Eleventy.js

    this.config.inputDir = this.inputDir;

    let formats = this.formatsOverride || this.config.templateFormats;
-    this.extensionMap = new EleventyExtensionMap(formats);
+    this.extensionMap = new EleventyExtensionMap(formats, config);


I could really use an example I think.

cmd.js

mariusschulz · 2020-11-23T15:54:05Z

@zachleat @MadeByMike I love this change! It'll be super useful for all kinds of async tasks done during the config setup, e.g. using Shiki (which requires async initialization) as a syntax highlighter.

Thanks for working on this! :)

kruncher · 2021-06-09T02:30:35Z

This feature would be extremely useful for me too.

AleksandrHovhannisyan · 2021-06-21T22:43:10Z

Agreed, this would be amazing! In particular, it would let you do really cool things like generating different favicon sizes at build time so that you don't have to do that with external tools. I don't think you can do this with the 11ty Image plugin unless you make it a shortcode since it's async.

(Edit: This is possible, disregard my old post.)

zachleat · 2022-01-03T18:52:05Z

Hey y’all, I feel bad about this but unfortunately (and this is entirely my fault) I need to close this one as stale. The timing ended up being really bad and the scope of this PR ended up being a huge contributor to its downfall, as I did a pretty sizable refactor of the configuration stuff shortly after this was opened.

Really sorry.

(Also cross linking to #2103)

zachleat · 2022-01-03T18:57:10Z

Another cross link to home base at #614

Make config run async

8d4c0ba

MadeByMike force-pushed the async-configs branch from 6dcbf0f to 8d4c0ba Compare September 27, 2020 06:19

MadeByMike added 2 commits September 27, 2020 16:45

Apply plugins in tests!

44d8e86

Fix, debug statement

634c1e0

MadeByMike added 2 commits September 28, 2020 09:15

Add plugins at the right time

25a621e

Merge branch 'async-configs' of https://github.com/MadeByMike/eleventy …

c2f230c

…into async-configs

Fix watcher

a26ad09

fix config test

32f501f

MadeByMike force-pushed the async-configs branch from 93348b0 to 32f501f Compare October 6, 2020 01:00

Only init config once

5918bab

Make templateConfig available to plugins

e84d7f1

MadeByMike force-pushed the async-configs branch from 6dd4374 to e84d7f1 Compare October 6, 2020 23:58

merge config in the correct order for plugins

34ca6fa

zachleat requested changes Oct 16, 2020

View reviewed changes

MadeByMike commented Oct 20, 2020

View reviewed changes

MadeByMike added 3 commits October 26, 2020 09:13

Update src/Eleventy.js

e145a60

remove note to mike

0097bb2

Move some of the set-up before init

3390d78

zachleat closed this Jan 3, 2022

zachleat mentioned this pull request Jan 3, 2022

Allow configuration to return a promise (async-friendly) #614

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make config run async #1429

Make config run async #1429

MadeByMike commented Sep 27, 2020 •

edited

Loading

MadeByMike commented Sep 27, 2020

MadeByMike commented Sep 28, 2020

MadeByMike commented Sep 28, 2020

MadeByMike commented Oct 6, 2020 •

edited

Loading

MadeByMike commented Oct 6, 2020

zachleat left a comment

zachleat Oct 16, 2020

MadeByMike Oct 18, 2020

MadeByMike Oct 18, 2020

zachleat Oct 16, 2020

zachleat Oct 16, 2020

MadeByMike Oct 18, 2020

MadeByMike Oct 18, 2020

MadeByMike Oct 18, 2020

MadeByMike Oct 18, 2020

MadeByMike Oct 18, 2020

MadeByMike Oct 18, 2020

MadeByMike Oct 18, 2020

mariusschulz commented Nov 23, 2020

kruncher commented Jun 9, 2021

AleksandrHovhannisyan commented Jun 21, 2021 •

edited

Loading

zachleat commented Jan 3, 2022

zachleat commented Jan 3, 2022


		this.templateData = new TemplateData(this.inputDir);
		this.templateData = new TemplateData(this.inputDir, config);

		return this.configOverride \|\| require("./Config").getConfig();
		return this.configOverride \|\| this.config;

Make config run async #1429

Make config run async #1429

Conversation

MadeByMike commented Sep 27, 2020 • edited Loading

MadeByMike commented Sep 27, 2020

MadeByMike commented Sep 28, 2020

MadeByMike commented Sep 28, 2020

MadeByMike commented Oct 6, 2020 • edited Loading

MadeByMike commented Oct 6, 2020

zachleat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariusschulz commented Nov 23, 2020

kruncher commented Jun 9, 2021

AleksandrHovhannisyan commented Jun 21, 2021 • edited Loading

zachleat commented Jan 3, 2022

zachleat commented Jan 3, 2022

MadeByMike commented Sep 27, 2020 •

edited

Loading

MadeByMike commented Oct 6, 2020 •

edited

Loading

AleksandrHovhannisyan commented Jun 21, 2021 •

edited

Loading