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

Refactored Drafts plugin #168

Closed
wants to merge 2 commits into from
Closed

Refactored Drafts plugin #168

wants to merge 2 commits into from

Conversation

johnwargo
Copy link

While studying the Drafts plugin for my own work, I realized that the current plugin had a lot more code than needed which affected readability and should have a slight impact on performance. This PR:

  • Replaces the environment variable with the isServing Boolean (which I think is faster in comparisons).
  • Moves the check for the serve or watch Boolean earlier in the comparison logic so if the build process starts with serve or watch, the code never even checks draft status because draft status doesn't matter at that point. This probably isn't a big deal, but I think it makes it clearer what's happening. With this refactored code, the same number of comparisons happen (I think).
  • Renames logged to isLogged making it clearer that it's a Boolean (for readability) and to align the variable name with isServing.
  • Moves the isLogged = true to the !isLogged check block so the value change only happens once instead of every time the watched site rebuilds. In the original, logged= true executed with every build which was unnecessary.
  • Reduces the code size from 50 lines to 34 (without removing any comments).

@zachleat
Copy link
Member

zachleat commented Oct 1, 2024

We simplified this one using the Preprocessor API (5 LoC):

eleventyConfig.addPreprocessor("drafts", "*", (data, content) => {
if(data.draft && process.env.ELEVENTY_RUN_MODE === "build") {
return false;
}
});

Thank you for the improvements though!

@zachleat zachleat closed this Oct 1, 2024
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