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 flag to run event handlers sequentially #3423

Conversation

vrugtehagel
Copy link
Contributor

@vrugtehagel vrugtehagel commented Aug 25, 2024

Resolves #3415.

This PR adds a flag to run event handlers sequentially, instead of in parallel. This is to avoid issues with plugins interfering with one another if they are doing asynchronous operations on the same set of files in parallel.

Copy link
Member

@zachleat zachleat left a comment

Choose a reason for hiding this comment

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

Looking great! I definitely don’t want to add a temporary CLI argument for this though—I think it’d be better to keep it as a Configuration API only.

Copy link
Member

@zachleat zachleat left a comment

Choose a reason for hiding this comment

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

I think just revert the changes in cmd.cjs and src/Eleventy.js and we’ll be good

@zachleat zachleat added enhancement feature: 🛠 configuration Related to Eleventy’s Configuration file labels Aug 30, 2024
@vrugtehagel
Copy link
Contributor Author

Alright, @zachleat - removed the CLI flag entirely and moved the setEmitterMode method to UserConfig. So essentially, now users would have the ability to do

export default function(eleventyConfig){
    eleventyConfig.setEmitterMode('sequential');
    // or…
    eleventyConfig.events.setHandlerMode('sequential');
} 

Hypothetically, we can remove the former in favor of the latter, but perhaps it's nice to have this "top-level" method since we have it for almost everything else. Let me know what you think :)

@zachleat
Copy link
Member

zachleat commented Sep 3, 2024

@vrugtehagel yeah I think in docs we’ll lean towards only recommending the former method—thank you!

Copy link
Member

@zachleat zachleat left a comment

Choose a reason for hiding this comment

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

LGTM thank you!

@zachleat zachleat added the needs-documentation Documentation for this issue/feature is pending! label Sep 3, 2024
@zachleat zachleat merged commit db2ef67 into 11ty:main Sep 3, 2024
9 checks passed
@zachleat
Copy link
Member

zachleat commented Sep 3, 2024

Shipping with 3.0.0-beta.2 and 3.0.0-alpha.19

@zachleat zachleat added this to the Eleventy 3.0.0 milestone Sep 3, 2024
@vrugtehagel vrugtehagel deleted the vrugtehagel/add-flag-to-run-event-handlers-sequentially branch September 4, 2024 13:13
zachleat added a commit to 11ty/11ty-website that referenced this pull request Sep 26, 2024
@zachleat
Copy link
Member

Temporary docs preview deploying here: https://11ty-website-git-v3-11ty.vercel.app/docs/events/#emitter-modes

@zachleat zachleat removed the needs-documentation Documentation for this issue/feature is pending! label Sep 26, 2024
DeeUnderscore added a commit to DeeUnderscore/11ty-website that referenced this pull request Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature: 🛠 configuration Related to Eleventy’s Configuration file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The eleventy.after event should not fire its handlers in parallel
2 participants