Skip to content
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

Add globalData via config #1280

Merged
merged 8 commits into from
Sep 1, 2020
Merged

Add globalData via config #1280

merged 8 commits into from
Sep 1, 2020

Conversation

MadeByMike
Copy link
Contributor

@MadeByMike MadeByMike commented Jun 23, 2020

Adds a new method to support adding of global data through config such as:

eleventyConfig.addGlobalData('function', () => new Date());

eleventyConfig.addGlobalData('async',  () =>
      new Promise((resolve) => {
        setTimeout(resolve, 100, "foo");
      })
);

eleventyConfig.addGlobalData('static', 'static');

Or chaining:

eleventyConfig
  .addGlobalData('function', () => new Date())
  .addGlobalData('async',  () =>
      new Promise((resolve) => {
        setTimeout(resolve, 100, "foo");
      })
  )
  .addGlobalData('static', 'static');

Date is a rather simple but useless example. Where this becomes more useful for me is sourcing initial global data from files stored above the inputDir. This solves #655 as you can import functions or an object from any file.

Where this becomes most interesting for me, is giving plugins the ability to provide data in addition to config. This opens up a whole range of possibilities one example might be an ecommerce plugin that sources data from an external API.

@MadeByMike
Copy link
Contributor Author

MadeByMike commented Jul 10, 2020

I've been using this in the wild for a few days now and I've learnt that what I often want to do is set multiple keys like this:

eleventyConfig.addGlobalData({
  buildTime: () => new Date(),
  somethingElse: 42
});

Any thoughts before I change it?

@MadeByMike
Copy link
Contributor Author

Ok I added the new format

src/TemplateConfig.js Outdated Show resolved Hide resolved
@zachleat
Copy link
Member

zachleat commented Sep 1, 2020

I do like this but I’m worried there is some ambiguity with the methods and how they relate to global data.

Specifically I’d like this to mimic a global data file.

eleventyConfig.addGlobalData({
    function: () => new Date(),
    async: async () =>
      new Promise((resolve) => {
        setTimeout(resolve, 100, "foo");
      }),
    static: "static",
});

This creates special new behavior for top level keys that are functions or promises in the object. Whereas this style:

eleventyConfig.addGlobalData("function", () => new Date());
eleventyConfig.addGlobalData("async", async () => new Date());
eleventyConfig.addGlobalData("static", "static");

Better matches how global data files work now. Does that make sense?

Here’s what might be confusing to me, consider the following global data file _data/myGlobal.js (that is similar to the first addGlobalData example above)

module.exports = {
    function: () => new Date(),
    async: async () =>
      new Promise((resolve) => {
        setTimeout(resolve, 100, "foo");
      }),
    static: "static",
};

With current Eleventy behavior the global data file would return the object without any function calls or promise resolving, right?

@zachleat zachleat self-requested a review September 1, 2020 14:54
@MadeByMike
Copy link
Contributor Author

Thanks @zachleat - reverted back to that signature format as suggested. My only concern here is I often have a plugin that does a bunch of stuff and then wants to add a big object to global data. Now I need to do this:

for (const [key, value] of Object.entries(stuffToAdd)) {
  eleventyConfig.addGlobalData(key, value);
}

A small bit of boilerplate that most plugins adding data will probably need. No big deal.

if (!this.globalData) {
let globalJson = await this.getAllGlobalData();

// OK: Shallow merge when combining rawImports (pkg) with global data files
this.globalData = Object.assign({}, globalJson, rawImports);
this.globalData = Object.assign(
Copy link
Member

Choose a reason for hiding this comment

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

Ah, sorry a few more thoughts: Not saying it does—just trying to think through it—but I wonder if this might need to work with deep merging? I think previously global data was just a shallow copy which has been fine because it’s first in the chain but we might get into some tricky situations. (After typing this all out—I think it’s probably fine to punt on this one for now but I just wanted to log it)

Related: As it stands, it looks like a global data file would overwrite the same key entry set in addGlobalData. Is this what we want or the other way around? I think addGlobalData will probably be plugin-land so I think global data files should probably have precedence. Okay, I’m done now—looks good!

@zachleat zachleat merged commit 65e02a3 into 11ty:master Sep 1, 2020
@zachleat
Copy link
Member

zachleat commented Sep 1, 2020

Shipping!

@zachleat zachleat added this to the Eleventy 1.0.0 milestone Sep 1, 2020
@zachleat zachleat mentioned this pull request Sep 1, 2020
@zachleat
Copy link
Member

zachleat commented Sep 1, 2020

Docs issue here #1386

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants