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

refactor(shortcodes)!: replace shortcodes with functions and filters #34

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dnnsjrng
Copy link
Member

To increase flexibility of the plugin the default shortcodes have been moved to an examples folder with could be imported optionally. This cleans up the project and reduces the complexity of the configuration object.

dnnsjrng added 2 commits June 27, 2022 22:34
…ow optionally

 There are example implementations for functions and filters available as well as proper documentation
@mvsde mvsde changed the title refactor(shortcodes): replace shortcodes with functions and filters refactor(shortcodes)!: replace shortcodes with functions and filters Jun 29, 2022
Copy link
Member

@mvsde mvsde left a comment

Choose a reason for hiding this comment

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

I wouldn’t call the example functions “examples” because they are totally legit functions and importing something with “example” in the name looks a bit weird in the Eleventy config file.

I propose the following:

Move the functions out of the examples folder and place them directly inside the functions folder.

Instead of deep importing them into the Eleventy config, add an exports map to the package.json:

"exports": {
  ".": "./index.js",
  "./functions/assetPath": "./lib/functions/examples/assetPath.js",
  "./functions/image": "./lib/functions/examples/image.js",
  "./functions/mix": "./lib/functions/examples/mix.js"
},

The functions can then be imported like this:

const functionAssetPath = require('@factorial/eleventy-plugin-twig/functions/assetPath')
const functionImage = require('@factorial/eleventy-plugin-twig/functions/image')
const functionMix = require('@factorial/eleventy-plugin-twig/functions/mix')

This creates a public API for the built-in functions and allows folder/file structure refactoring without the need for a breaking change.

Comment on lines +24 to +25
- Use **functions** and [`twig.extendFunction()`](https://twig.symfony.com/doc/2.x/advanced.html#functions) to extend Twig with custom functions
- Use **filters** and [`twig.extendFilter()`](https://twig.symfony.com/doc/2.x/advanced.html#filters) to extend Twig with custom filters
Copy link
Member

Choose a reason for hiding this comment

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

twig.extendFunction() and twig.extendFilter() is done automatically by the plugin. I don’t think it is necessary to mention the “internals” here.

Comment on lines +30 to +31
- **[Responsive Images](lib/functions/README.md)**: Uses [`@11ty/eleventy-img`](https://github.com/11ty/eleventy-img) plugin to autogenerate responsive images
- **[Hashed Assets](lib/functions/README.md)**: If you have generated a manifest (e.g. with [`@factorial/eleventy-plugin-fstack`](https://github.com/factorial-io/eleventy-plugin-fstack)) you could let Eleventy replace unhashed assets like `css/js` automatically with their hashed versions
Copy link
Member

Choose a reason for hiding this comment

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

You can link directly to the headings with:

  • [Responsive Images](lib/functions/README.md#image)
  • [Hashed Assets](lib/functions/README.md#mix)

*/

/**
* @typedef {object} TWIG_OPTIONS
* @property {SHORTCODE[]} [shortcodes] - array of shortcodes
* @property {Function[]} [functions] - array of functions to extend twig
Copy link
Member

Choose a reason for hiding this comment

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

{FUNCTION[]} should be uppercase.

Comment on lines +105 to +110
filters: [
// see filters/README.md
],
functions: [
// see functions/README.md
],
Copy link
Member

Choose a reason for hiding this comment

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

The filters and functions (and readmes) are nested inside lib/….


## Side note:

Please read the [Twig](https://twig.symfony.com/doc/2.x/advanced.html#extending-twig) Documentation about how to extend twig properly first. Checkout the [Twig.js] documentation for `extendFunction()` as well. Also notice that functions should be used for content generation and frequent use whereas filters are more for value transformations in general.
Copy link
Member

Choose a reason for hiding this comment

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

The “extending Twig” link points to version 2.x. The latest Twig version is 3.x.

* @param {import("../plugin").USER_OPTIONS} userOptions
*/
module.exports = (eleventyConfig, userOptions) => {
(userOptions.twig?.filter || []).forEach((filter) => {
Copy link
Member

Choose a reason for hiding this comment

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

functions is using the plural. For consistency the filters should be located at userOptions.twig.filters.

*/

/**
* @typedef {object} TWIG_OPTIONS
* @property {SHORTCODE[]} [shortcodes] - array of shortcodes
* @property {Function[]} [functions] - array of functions to extend twig
Copy link
Member

Choose a reason for hiding this comment

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

{FUNCTION[]} should be uppercase.

There is an example implementation in `examples/mix.js`. To activate that implementation you have to add this module to the `userOptions.twig.functions` in your Eleventy configuration file and define the `userOptions.mixManifest` as well as the `userOptions.dir.output` property optionally like:

```js
const mix = require("@factorial/eleventy-plugin-twig/functions/examples/mix");
Copy link
Member

Choose a reason for hiding this comment

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

The module path is missing lib: @factorial/eleventy-plugin-twig/lib/functions/examples/mix.js.

There is an example implementation in `examples/assetPath.js`. To activate that implementation you have to add this module to the `userOptions.twig.functions` in your Eleventy configuration file and define the `userOptions.assets.base` property like:

```js
const assetPath = require("@factorial/eleventy-plugin-twig/functions/examples/assetPath");
Copy link
Member

Choose a reason for hiding this comment

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

The module path is missing lib: @factorial/eleventy-plugin-twig/lib/functions/examples/assetPath.js.

You can find an example implementation in `examples/image.js`. To activate that implementation you have to add this module to the `userOptions.twig.functions` in your eleventy configuration file, as well as a couple of other necessary properties like:

```js
const image = require("@factorial/eleventy-plugin-twig/functions/examples/image");
Copy link
Member

Choose a reason for hiding this comment

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

The module path is missing lib: @factorial/eleventy-plugin-twig/lib/functions/examples/image.js.

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

Successfully merging this pull request may close these issues.

2 participants