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 beforeOnce and afterOnce events for single execution use cases #2964

Open
johnwargo opened this issue Jun 15, 2023 · 6 comments
Open

Add beforeOnce and afterOnce events for single execution use cases #2964

johnwargo opened this issue Jun 15, 2023 · 6 comments

Comments

@johnwargo
Copy link

johnwargo commented Jun 15, 2023

Is your feature request related to a problem? Please describe.

I created a node module to generate paginated category pages for a site (https://www.npmjs.com/package/eleventy-generate-category-pages). When I run an Eleventy build using --watch or --serve, it creates an infinite loop during the build process where regenerating the category pages causes a site rebuild.

What I need for this is a beforeOnce event so the category page generation only happens once, at the beginning of the --serve process.

Describe the solution you'd like

Something like this:

var beforeFirstRun = true;
if (beforeFirstRun) {
  beforeFirstRun = false;
  await this.config.events.emit("eleventy.beforeOnce", eventsArg);
}

await this.config.events.emit("beforeBuild", eventsArg);
await this.config.events.emit("eleventy.before", eventsArg);

we can do the same for after, an afterOnce event:

var afterFirstRun = true;
if (afterFirstRun) {
  afterFirstRun = false;
  await this.config.events.emit("eleventy.afterOnce", eventsArg);
}

await this.config.events.emit("afterBuild", eventsArg);
await this.config.events.emit("eleventy.after", eventsArg);

Describe alternatives you've considered

To solve this, I added a similar check in my eleventy.before listener:

var firstRun = true;
eleventyConfig.on('eleventy.before', async ({ dir, runMode, outputMode }) => {
  if (firstRun) {
    firstRun = false;
    generateCategoryPages({}, true, false);
  }
});

Additional context

I can add this as a PR, buut wanted to see what the community thinks of it first. and, I'd have to figure out how to write a test for this.

@Snapstromegon
Copy link
Member

Your plugin looks a lot like Collections to me (or at least the same thing is doable with collections).

I personally think that a rebuild using watch and serve should always rebuild everything and no permanent output should be created in the input folder from a plugin. Instead I think something like this plugin should either provide the data in a js data file, or add the data as global data into the eleventy build.

@johnwargo
Copy link
Author

My plugin generates a file you can update with category descriptions then display them on a page. Descriptions are maintained between generations. You can't do that with collections. It also creates separate pages per category which allows you to use pagination per category, something else you can't do with collections as pagination doesn't support nested paginations.

If you rebuild during watch, the site goes into an infinite loop - because my plugin creates new category files. During rebuild, I create new category docs and the build kicks off again - infinite loop. a very bad UX.

@Snapstromegon
Copy link
Member

You're right, nested pagination is something that is still a missing feature from 11ty and requested in #955.

Regarding the descriptions: I'd probably just create one static data file which just contains a mapping of tag name to description.

Either way, I think you should still fix your infinite loop. One idea would be to read the content of the file before writing new content and only write to a file if you actually need to change it. Mobing the generation to beforeOnce or something similar will break your plugin if a user adds a category while a --watch is running.

@johnwargo
Copy link
Author

@Snapstromegon you're missing the point. I didn't suggest this feature in an effort to get someone to suggest another approach to me. I identified a very specific scenario that is giving me a problem and I proposed a potential solution I assumed other would be interested in as well.

I don't want to maintain a static data file, I want my site to update the file automatically every time I add a category.

Is there something wrong with my proposal? Is there a technical reason this would be a bad or harmful feature in Eleventy? You can object to my proposal on those grounds if you want, I guess, but proposing alternatives isn't helping.

@Snapstromegon
Copy link
Member

@johnwargo I guess eleventy.beforeOnce would be okay to add. Do you think it should be executed before or after eleventy.beforeWatch?
I still think that the build itself should never care whether it's the first or fifth build. There are reasons to do things once though for either performance reasons or because you have to do special things around watch/serve to e.g. create file watchers.

I still think adding eleventy.afterOnce would be harmful to the project because IMO

  • it encourages bad practices like giving the first build sideeffects
  • it is not clear from the name when it runs (could be solved by e.g. using elevent.afterFirstBuild)
  • it's an easy source for unexpected errors when it's used (especially by plugins) without enough care (e.g. nothing done in a hook that's just run once, should IMO depend on any input from the project) which can be especially hard for new developers

In general, both events are right now already available via the workaround you mentioned in your original comment.

More specifically to your usecase:
I strongly believe, that a build should under no circumstances modify any input to the build. It should only touch the output and optionally caches and usually shouldn't even make writing/updating API calls. If a build does these things, from my experience it will lead to way bigger problems in the future.

Regarding proposing alternatives:
I'm sorry if this was not what you expected, but I can only comment based on what I have seen in this thread and the other stuff around 11ty and it is very common that people open issues here for enhancements, even though there are significantly easier and cleaner solutions for their problems. Proposing alternatives is often considered helpful (from what I hear here and on the community Discord), since it brings a speedy alternative even if the change is implemented at some point.

@johnwargo
Copy link
Author

johnwargo commented Feb 19, 2024

@Snapstromegon thank you, that actually addressed my specific request. That was excellent feedback and a great analysis and arguments. That's what I expected in your first response.

My apologies for coming back at you that way but so many people don't give these types of things careful consideration instead simply coming back with "well you should rewrite your code instead of doing this feature" like you did.

I have more than 40 years experience writing software (for both personal use and commercial purposes) and I spent a lot of time thinking through what I wanted to do in my particular case. My particular site is 15 years old (I migrated it form Joomla) and has 10, 20, 30 or more posts in some categories. The site must have a pagination page per category and maintaining individual pages manually is a pain as I add categories from time to time and don't want to have to remember to create a new page when I do. My module takes care of all of that for me.

You're probably right, afterOnce doesn't make sense, I was just being thorough in my post just in case others had a use case for the after option.

In my mind beforeOnce should execute before beforeWatch but I honestly don't know anything about beforeWatch.

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

No branches or pull requests

2 participants