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

Expose inputPath in transforms #789

Closed
zachleat opened this issue Nov 24, 2019 · 21 comments
Closed

Expose inputPath in transforms #789

zachleat opened this issue Nov 24, 2019 · 21 comments

Comments

@zachleat
Copy link
Member

Linters support an inputPath argument but transforms do not

https://www.11ty.io/docs/config/#transforms
https://www.11ty.io/docs/config/#linters

Requested by @nhoizey https://twitter.com/nhoizey/status/1197932949708165120

@zachleat zachleat added enhancement needs-votes A feature request on the backlog that needs upvotes or downvotes. Remove this label when resolved. labels Nov 24, 2019
@zachleat
Copy link
Member Author

This repository is now using lodash style issue management for enhancements. This means enhancement issues will now be closed instead of leaving them open.

View the enhancement backlog here. Don’t forget to upvote the top comment with 👍!

@nhoizey
Copy link
Contributor

nhoizey commented Nov 25, 2019

If anyone needs this, until it is supported I use a little trick:

I put the inputPath in a data-attribute of the article element in the page's generated HTML:

data-input="{{ page.inputPath }}"

I then need to use jsdom (or regexes) to get the value inside the transform, but at least I get the value.

@nhoizey
Copy link
Contributor

nhoizey commented Dec 11, 2019

Actually, I need both the inputPath and outputPath (even if the doc says "You probably won’t use this" 😅) in the transform's context.

I'm currently passing it from the template:

https://github.com/nhoizey/nicolas-hoizey.com/blob/b0355e14d03b322bd425ed8ec4baa0e968f57081/src/_layouts/default.njk#L10-L11

<body
    
    data-img-src="{{ page.inputPath | dirname }}/"
    data-img-dist="{{ page.outputPath | dirname }}/"
  >

To the transform:

https://github.com/nhoizey/nicolas-hoizey.com/blob/d01079d81694f23050162ee89ca0f1951c0683ed/src/transforms/cloudinary-transform.js#L44-L50

// Get images src and dist path from attributes on body
// TODO: move to context data
let documentBody = document.querySelector('body');
let srcPath = documentBody.getAttribute('data-img-src');
let distPath = documentBody.getAttribute('data-img-dist');
documentBody.removeAttribute('data-img-src');
documentBody.removeAttribute('data-img-dist');

@Snugug
Copy link
Contributor

Snugug commented Jan 14, 2020

https://www.npmjs.com/package/eleventy-plugin-local-respimg would benefit greatly from this as well

@nhoizey
Copy link
Contributor

nhoizey commented Mar 20, 2020

I'm still using the trick in #789 (comment)

My situation is the following:

I developed a plugin: eleventy-plugin-images-responsiver. It adds a transform.

NB: Here, I'm not even sure I pass the plugin options as I should, it feels strange using a global variable for that: https://github.com/nhoizey/eleventy-plugin-images-responsiver/blob/master/.eleventy.js

Anyway, this transform calls an imageResponsiver function that comes from another npm module: images-responsiver. This function doesn't need inputPath and outputPath.

But I need to be able to add inputPath and outputPath to the options (here imagesResponsiverOptions) before calling it, because this function calls other hook functions that I define in the plugin options, when adding it in my Eleventy config:

const imagesResponsiver = require('eleventy-plugin-images-responsiver');
const imagesResponsiverConfig = require('./src/_data/images-responsiver-config.js');
eleventyConfig.addPlugin(imagesResponsiver, imagesResponsiverConfig);

The imagesResponsiverConfig object comes from _data/images-responsiver-config.js.

It contains the runBefore and runAfter members, hook functions to run before and after the core of the imageResponsiver function.

In my current case, the runBefore function needs both inputPath and outputPath, and get them from the DOM of the transformed page:

let srcPath = documentBody.getAttribute('data-img-src');
let distPath = documentBody.getAttribute('data-img-dist').replace(/^dist/, '');

If inputPath was available as a parameter to the transform, I could add it to the imagesResponsiverOptions object for these hooks to get it.

NB: I could already do it for outputPath, but I preferred having the same "passage way" for both variables.

Thanks for reading all of this, if you did… 😅

@zachleat
Copy link
Member Author

Really sorry, this is getting punted to 1.0.

@zachleat zachleat modified the milestones: Eleventy v0.11.0, Eleventy v1.0.0 May 10, 2020
@nhoizey
Copy link
Contributor

nhoizey commented May 10, 2020

@zachleat no problem on my side, I have a work around which can last as long as you need.

@zachleat
Copy link
Member Author

The awkward part of this is that the linter and transform arguments are going to be reversed. Hmm.

@Snugug
Copy link
Contributor

Snugug commented May 16, 2020 via email

@nhoizey
Copy link
Contributor

nhoizey commented May 16, 2020

I agree they should not be reversed for the future, and breaking it right know, before 1.0 (or for it) is better than later.

@daKmoR
Copy link

daKmoR commented May 25, 2020

imho best would be to have an object for options 🤔

eleventyConfig.addLinter("my-linter", (content, { inputPath, outputPath }) => {});

that way the order doesn't matter and additional info can get added without a breaking change.

PS: in my experience an options object generally better for optional infos

@daKmoR
Copy link

daKmoR commented Jun 20, 2020

any way we can help to make this happen? I mean the code change is "trivial" - so what info do we need to proceed?

we now patch it for our use case... but that is of course not a long term solution 😅

@TigersWay
Copy link
Contributor

It seems this one is needed in multiple places, breaking change(s) or not!!

@gregives
Copy link

gregives commented Aug 2, 2020

In a transform, can you not access the inputPath already, via this.inputPath? For example,

eleventyConfig.addTransform('myTransform', function(content, outputPath) {
    console.log(this.inputPath)
})

(Not a long-term solution, I just wondered if this would work fine for now)

@TigersWay
Copy link
Contributor

Just to add a little something...

  • First of all, thanks to @nhoizey for his neat trick,
  • and then I had to go further, as I have - sometimes - multiple posts in one single page. It means one only inputPath is not so interesting anymore, I went to add that data-path to each article.

@daKmoR
Copy link

daKmoR commented Aug 5, 2020

I actually need a different this in the function... but yeah it's now like that

const that = this;
this.elev.config.filters['hook-for-rocket'] = function hook(html, outputPath) {
  const { inputPath } = this;
  return that.doSomething(inputPath);
};

thx for the workaround 🤗

but yeah no workaround would be better 😬 so fixing this would still be nice 👍

@nhoizey
Copy link
Contributor

nhoizey commented Aug 11, 2020

@TigersWay you should thank @gregives instead… 😅

@zachleat
Copy link
Member Author

Code is in for this. We’re using this.inputPath and this.outputPath to avoid the argument order consistency problem with linters. We’ll keep support for the existing arguments but transition away from them in the docs.

More importantly, since it seems like y’all are using them here (specifically #789 (comment)): the object key filters is going away in 1.0 and will throw a contextual error message if you use it. It’s been deprecated since Eleventy 0.3.3 😅

Hopefully that workaround won’t be necessary anyway with this fix.

@zachleat zachleat modified the milestones: Planning for Eleventy v1.0.0, Eleventy 1.0.0 Oct 30, 2020
@zachleat zachleat removed the needs-votes A feature request on the backlog that needs upvotes or downvotes. Remove this label when resolved. label Oct 30, 2020
@zachleat
Copy link
Member Author

🚢🛳⛴🚀📦

@zachleat
Copy link
Member Author

Example of the error messaging:

image

@nhoizey
Copy link
Contributor

nhoizey commented Oct 30, 2020

That's awesome, thanks a lot Zach! 👍

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.

6 participants