eleventy-plugin-amp
eleventy-plugin-amp copied to clipboard
ampproject
Avoid modifying outputDir
I think this line should be removed. (It was added via #31.)
I think the cached images should be created directly in outputDir rather than ${outputDir}/${urlPath}, both for semantic reasons and also because this is how it's documented in the eleventy-img config.
Why does this need to be removed? outputDir is the general output dir (_site by default). This is used by other features as well, so it makes more sense to specify the general output dir instead of separate outputdirs for each feature. urlPath gives the option to customize the image folder.
One reason I think this should be avoided is that the concatenation mixes together different "types": urlPath is in the URL space, outputDir is in the filesystem space, so joining them together is slightly weird. And it actually does the wrong thing if urlPath is something like https://cdn.example.com/images/. (Sure, "path" is in the name, but this does otherwise work.)
Another issue is, if more options will be simply passed through to eleventy-img in the future, eleventy-plugin-amp will forever need to document that outputDir is transformed before being passed to eleventy-img, it's not possible to simply link to the eleventy-img docs.
Yeah, that's tricky and I'm not sure what the right way forward is. My other concern is: Is it a good idea to pass through eleventy-img options? This makes it harder to switch to a different library in the future. Currently, parameters are passed through, but it's not publicly documented.
What would be your suggestion?
I think I'd be happy with eleventy-plugin-amp staying out of the way and passing through as much as possible, even if this means users of the plugin need to configure the same thing twice. (e.g. if two different libraries used by the plugin both require an output directory.)
It's true that this ties the plugin to its implementation details, but maybe that's okay.
Also, in my short time with the plugin, this is the second time I've hit a case where the options not being passed through has been a bit confusing. (I've been trying to get the plugin to generate self-hosted AMP components but so far I haven't been able to figure out how to thread ampUrlPrefix through to the optimizer's config.)
I've been trying to get the plugin to generate self-hosted AMP components
There's been quite a few problems with this (I've been working on the same). It should work now (see https://github.com/ampproject/eleventy-plugin-amp/pull/35/).
