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

outputPath.endsWith check fails with permalink: false #653

Open
pepelsbey opened this issue Aug 10, 2019 · 13 comments
Open

outputPath.endsWith check fails with permalink: false #653

pepelsbey opened this issue Aug 10, 2019 · 13 comments
Labels
needs-discussion Please leave your opinion! This request is open for feedback from devs.

Comments

@pepelsbey
Copy link

pepelsbey commented Aug 10, 2019

Describe the bug

When you have files with permalink: false then outputPath.endsWith check fails.

if(outputPath.endsWith('.html')) {  }
> Having trouble writing template: false (TemplateWriterWriteError)
> outputPath.endsWith is not a function (TypeError):

To Reproduce

  1. Create a file with permalink: false that resolves into *.html
  2. Add a transform, just like in this example.
  3. Build

Expected behavior

Successful build.

Environment:

  • OS and Version: macOS 10.14.6
  • Eleventy Version: 0.8.3

Additional context

It could be fixed by checking if outputPath is available:

if(outputPath && outputPath.endsWith('.html')) {  }

…or by changing the outputPath behavior, which is preferable :)

@zachleat
Copy link
Member

Hmmmmmmmmmmmmmmmmmm… Should we solve this in the library? Transforms should run on permalink: false templates. What would be a better value for outputPath there?

@zachleat zachleat added needs-discussion Please leave your opinion! This request is open for feedback from devs. and removed needs-triage labels Aug 15, 2019
@pepelsbey
Copy link
Author

pepelsbey commented Aug 15, 2019

How about if (!permalink) { outputPath = '' }? Pseudo code, of course.
It’s empty, but could be checked with endsWith.

@zachleat
Copy link
Member

zachleat commented Aug 15, 2019

Hmm, this makes me a little uncomfortable because it’s a little bit of magic? Right now permalink: false means outputPath: false, which makes the most sense and has the least amount of magic.

Not saying no, just want to think it out all the way.

One other question that comes to mind: if you set permalink: should it be the same as permalink: false? Does permalink: have any unique functional value?

@pepelsbey
Copy link
Author

You could require permalink key to accept strings, not Boolean values: permalink: '' or… Even better: keep it they way it is (string or false), but transform it to string, while parsing the metadata. Yes, a little bit of magic. Storing false as '' would keep API the same, but it would also make the outputPath check work. Personally, I don’t see why front matter key permalink and internal API property outputPath should have the same type. If permalink is false then outputPath is empty. It makes perfect sense to me. The first one is a yes/no thing, the second one is always a path, but sometimes it’s empty.

@edwardhorsford
Copy link
Contributor

I just came across this - added the html minification example to my eleventyConfig and it blew up.

Two things caused this:

  • I had some content in my site using ' and " such that the html was actually invalid - this is the first time I've noticed. I've now put more safeguards to make sure my content converts those to smartquotes before they get to the transform stage.
  • One output path out of 722 was a boolean rather than string. The 'content' was an empty array - so I'm not quite sure what it is.

I'm not sure the transform necessarily needs to so something differently, but I think the example code for minification should handle this case. Something as simple as testing for a string / url and then proceeding. It using endsWith() strongly implied that outputPath would always be a string - which is not the case.

@davidysoards
Copy link

For anyone coming across this by using the html-minifier example in the docs and a link to an outside url using permalink: false the solutions is pretty simple, just add a check for outputPath to the if statement

if (outputPath && outputPath.endsWith('.html'))

peterjcaulfield pushed a commit to peterjcaulfield/eleventy-plugin-helmet that referenced this issue Feb 6, 2021
vseventer added a commit to vseventer/eleventy-plugin-helmet that referenced this issue Feb 14, 2021
* outputPath is not guaranteed to be a string when permalink is set to false.

See: 11ty/eleventy#653

* Favour optional chaining

Co-authored-by: Mark van Seventer <mark@vseventer.com>

Co-authored-by: Peter <peterc@optum.com>
Co-authored-by: Mark van Seventer <mark@vseventer.com>
@jdvivar
Copy link

jdvivar commented Mar 3, 2021

What if the outputPath has no html file extension? As in:

---
permalink: "whatever"
---

How would a transform plugin for HTML would know the file to transform is HTML prior to the transformation itself?

@jdvivar
Copy link

jdvivar commented Mar 3, 2021

I think I found a good balance in my plugin to deal with both undefined file extensions and permalink false:
jdvivar/eleventy-plugin-add-web-component-definitions@286ecef

@igelstorm
Copy link

I also came across this issue by following the html-minifier transform example in the docs. I fixed it by changing the code as @davidysoards suggested (thanks!).

It seems to me that the most immediate problem is not necessarily that there's anything unintuitive about the API, but that the docs contain a code example that breaks fairly easily (i.e. whenever any template has permalink: false). So maybe the appropriate fix is to amend that example per @davidysoards's suggestion? And potentially also clarify in the docs (e.g. here) that outputPath isn't guaranteed to be a string.

@davidysoards
Copy link

@igelstorm you're welcome! glad it helped someone.

@pdehaan
Copy link
Contributor

pdehaan commented Mar 24, 2021

I think the docs page is at https://github.com/11ty/11ty-website/blob/master/docs/config.md, if somebody want to create a Pull request for this.

@igelstorm
Copy link

igelstorm commented Mar 24, 2021

I have opened a PR to fix the minification example and clarify this behaviour in a few other places in the docs: 11ty/11ty-website#991

Comments welcome from anyone who encountered this issue about whether these changes would've been enough to help them at the time!

@SergeyRe
Copy link

SergeyRe commented Sep 25, 2021

My temporary and not smart solution for such case with AMP-plugin is to not use false value for permalink and generate empty page with no collision for plugin

---js
{
  eleventyComputed:{
  permalink: data=>{return (data.graphql.length>1 ? "index.html" : "empty")}  
  }
}
---

{% if permalink!="empty" %}

{% endif %}

chalkygames123 added a commit to chalkygames123/front-end-template that referenced this issue Mar 25, 2023
chalkygames123 added a commit to chalkygames123/front-end-template that referenced this issue Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-discussion Please leave your opinion! This request is open for feedback from devs.
Projects
None yet
Development

No branches or pull requests

8 participants