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

The eleventy.after event should not fire its handlers in parallel #3415

Closed
vrugtehagel opened this issue Aug 15, 2024 · 2 comments · Fixed by #3423
Closed

The eleventy.after event should not fire its handlers in parallel #3415

vrugtehagel opened this issue Aug 15, 2024 · 2 comments · Fixed by #3423

Comments

@vrugtehagel
Copy link
Contributor

vrugtehagel commented Aug 15, 2024

Operating system

macOS Sonoma 14.6.1

Eleventy

3.0.0-beta.1

Describe the bug

Eleventy should not fire its eleventy.after (and probably also not its eleventy.before) event handlers in parallel. This is an issue, because any plugin that manipulates files asynchronously now directly conflicts with another, introducting race conditions and potential flaky error scenarios.

For example, I have written a plugin that adds hash parameters to referenced asset URLs. If I were to use it in combination with any plugin that moves or renames files, the ouput would be a mangled mess of half-combinations of files that have moved, some with hash parameters, some without, and potentially duplicate files because one plugin wrote to a file that the other has just moved.

In a nutshell, the AsyncEventEmitter should use a simple for-loop instead of Promise.all() to run the handlers sequentially, though potentially we'd want an (internal) option for it to determine whether to run handlers in parallel or not for each individual event type.

@zachleat
Copy link
Member

Hmm! I think I’m okay with this but it’s too late for this default to ship with 3.0. We could do a temporary feature flag to ease into it.

@vrugtehagel
Copy link
Contributor Author

Cool, I'll draft up a PR for it!

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

Successfully merging a pull request may close this issue.

2 participants