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

Setting front matter variable permalink: false results in a compilation error #4

Open
unconfident opened this issue Nov 6, 2020 · 4 comments · May be fixed by #5 or #11
Open

Setting front matter variable permalink: false results in a compilation error #4

unconfident opened this issue Nov 6, 2020 · 4 comments · May be fixed by #5 or #11
Labels
bug Something isn't working

Comments

@unconfident
Copy link

This code leads to problems in some situations when permalink is customized by the user.

const url = get(page, 'url', '');
const contextLocale = url.split('/')[1];
const locale = localeOverride || contextLocale;

Here are couple of examples:

1. permalink changed to a file in top level directory:

---
# file src/en/404.md
layout: default
permalink: 404.html
---
# Error
Page not found 

Locale becomes undetermined, even though page is placed in the "en" directory, Will be using fallback locale if specified

2. permalink set to false

Quoting original 11ty documentation here:

If you set the permalink value to be false, this will disable writing the file to disk in your output folder. The file will still be processed normally (and present in collections) but will not be available in your output directory as a standalone template.

In this case compilation crashes with error: TypeError: url.split is not a function

Proposed solution:

  • Rely on locale variable. It's already suggested by the Readme to create directory data file (i.e. src/en/en.json) with following contents
    {
      "dir": "ltr",
      "lang": "en",
      "locale": "en"
    }
    could just reuse that if defined
  • Or, if you still prefer parsing it from page url I suggest to rely on the source file structure instead. Make use of page.filePathStem or page.inputPath instead of page.url (see 11ty documentation) for parsing locale
@unconfident unconfident linked a pull request Nov 6, 2020 that will close this issue
@adamduncan adamduncan added the bug Something isn't working label Nov 21, 2020
@adamduncan
Copy link
Owner

Thanks for the bug report, @unconfident.

I understand the 2nd error case, and agree that using filePathStem is the way to go to avoid that error 👍

I'm not sure I follow the 1st scenario. If the 404 page was to be "placed in the en directory," should its permalink be /en/404.html (or could be permalink: /{{ locale }}/404.html to automatically inherit locale), as opposed to outputting it to the top-level? In which case does the plugin behave as expected?

@unconfident
Copy link
Author

Oh, sorry, I forgot to respond to your message.

I'm not sure I follow the 1st scenario. If the 404 page was to be "placed in the en directory," should its permalink be /en/404.html (or could be permalink: /{{ locale }}/404.html to automatically inherit locale), as opposed to outputting it to the top-level? In which case does the plugin behave as expected?

Error pages are in many cases returned by the web server or maybe caching proxy / CDN and thus can only be served in one locale anyway. So I saw no reason to generate them for every language if only one is ever going to be used. With this in mind moving it a top level felt like a natural thing to do. I can name you at least three reasons why this can be desirable:

  • I feel like this would make results of code generation more predictable and obvious which is hopefully going to make deployments more straightforward, especially when 3rd party is involved (i.e. when Dev Ops team/guy is in charge of it).
  • Unlike documenting or telling somebody "configure to serve /en/404.html as Not Found page" not having it mapped 1:1 to source file also leaves control over which content is being served to the users in the developer's hands.
  • It allows for url schema change in the future without need to reconfigure whatever-is-serving-error-pages

Because parts of the shared layout still depended on the i18n plugin and expect to find some localization variables I figured I would have to place the source file for the error page in the language specific directory and just swap the output path. You know the rest.

@adamduncan adamduncan linked a pull request Mar 4, 2021 that will close this issue
@adamduncan adamduncan linked a pull request Mar 4, 2021 that will close this issue
@adamduncan
Copy link
Owner

adamduncan commented Mar 4, 2021

Thanks for the additional info @unconfident. That helps me to better understand.

Had created a hotfix for the permalink: false issue alone for now (#11) as I'm a little apprehensive to introduce regex on the directory locale prefix. I.e. We suggest the convention of ISO-3166 alpha-2 (with or without ISO-639 language code), but in theory users could use alpha-3 codes/full country names, etc. Hard to get away from the fact it's a brittle approach (which says something about the overly-optimistic assumption I made originally).

Leads me to a question: #11 (comment)

@tylermercer
Copy link

Not sure if this package is maintained anymore, but thought I would post this here in case anyone else runs into this: I used patch-package to apply the filePathStem fix and it works great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants