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.

Sign up for GitHub

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

Jump to bottom

chore: avoid generating subdirectories for each page on new docs site #15967

Merged
merged 6 commits into from Jun 10, 2022
Merged

chore: avoid generating subdirectories for each page on new docs site #15967

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ff722ab
chore: avoid generating subdirectories for each page on new docs site
mdjermanovic Jun 5, 2022
1e2a42c
add comments
mdjermanovic Jun 5, 2022
587511f
refactor pathPrefix
mdjermanovic Jun 5, 2022
c5ba8c1
update component library permalinks
mdjermanovic Jun 6, 2022
59c867b
rewrite URLs
mdjermanovic Jun 10, 2022
8f6793a
fix indentation in package.json
mdjermanovic Jun 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 39 additions & 2 deletions docs/.eleventy.js
View file
Open in desktop
Expand Up @@ -19,10 +19,10 @@ module.exports = function(eleventyConfig) {
* The site is loaded from /docs on eslint.org and so we need to adjust
* the path prefix so URLs are evaluated correctly.
*
* The path prefix is turned off for deploy previews so we can properly
* The path prefix is turned off for `npm start` and deploy previews so we can properly
* see changes before deployed.
*/
const pathPrefix = process.env.CONTEXT === "deploy-preview" ? "" : "/docs";
const pathPrefix = ["local-serve", "deploy-preview"].includes(process.env.CONTEXT) ? "" : "/docs";
Copy link
Member

Choose a reason for hiding this comment

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

We should use the pathPrefix locally too, otherwise, we won't be able to tell when the url filter isn't used. The way I set it up was so that these errors would be obvious locally because we won't get them in deploy preview.

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't make it work (see the third point in "What changes did you make" in the original post). I'll see if the browser-sync config override can be done differently.

Copy link
Member

Choose a reason for hiding this comment

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

Huh. I haven’t had any trouble since I first added pathPrefix. Eleventy generates an index.html file at the root that redirects to /docs and then everything works perfectly for me. I’m using the same setup on the playground.

Copy link
Member Author

Choose a reason for hiding this comment

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

Eleventy's built-in server returns the content of foo/index.html when we request foo/, but it doesn't return the content of foo/bar.html when we request foo/bar, which is what we'll have after this change. It needs some additional configuration.

Without pathPrefix, it works well when I add serveStaticOptions: { extensions: [ 'html' ] } to the config that eleventy passes into the underlying server.

eleventyConfig.setBrowserSyncConfig({
    server: {
        baseDir: "_site",
        serveStaticOptions: {
            extensions: ["html"]
        }
    }
});

With pathPrefix, for some reason it doesn't work.

eleventyConfig.setBrowserSyncConfig({
    server: {
        baseDir: "_site/_eleventy_redirect",
        routes: {
            "/docs": "_site"
        },
        serveStaticOptions: {
            extensions: ["html"]
        }
    }
});
Cannot GET /docs/user-guide/configuring/configuration-files

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried with rewriting URLs instead of configuring the server. That seems to work well so I restored pathPrefix for local development.


//------------------------------------------------------------------------------
// Filters
Expand Down Expand Up @@ -91,6 +91,19 @@ module.exports = function(eleventyConfig) {
return markdown.render(value);
});

/*
* Removes `.html` suffix from the given url.
* `page.url` will include the `.html` suffix for all documents
* except for those written as `index.html` (their `page.url` ends with a `/`).
*/
eleventyConfig.addFilter("prettyURL", url => {
Copy link
Member

Choose a reason for hiding this comment

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

I’m concerned about this approach. I don’t think it’s at all clear where we should and should not be using this filter, and I fear this will cause more errors in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

The rule would be to use this filter in all places where page.url is written into the output content. In places where it is used in calculations, there is no need to use the filter (example on the current website). This is the same approach as on the current website.

If you have an idea for another approach, I could try it. We could maybe write the files without .html extension, but that would probably require additional configuration on Netlify to treat extensionless files as html.

if (url.endsWith(".html")) {
return url.slice(0, -".html".length);
}

return url;
});

//------------------------------------------------------------------------------
// Plugins
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -345,6 +358,30 @@ module.exports = function(eleventyConfig) {

// END, eleventy-img

//------------------------------------------------------------------------------
// Settings
//------------------------------------------------------------------------------

/*
* When we run `eleventy --serve`, Eleventy 1.x uses browser-sync to serve the content.
* By default, browser-sync will not serve `foo/bar.html` when we request `foo/bar`.
* Thus, we need to specify "html" in `server.serveStaticOptions.extensions` so that
* pretty links without `.html` can work in a local development environment.
*
* Note that eleventy is doing a shallow merge into its own browser-sync config,
* so this will unfortunately completely overwrite `server` settings.
* https://github.com/11ty/eleventy/blob/v1.0.1/src/EleventyServe.js#L78-L91
* Therefore, we also have to specify `baseDir` here.
*/
eleventyConfig.setBrowserSyncConfig({
server: {
baseDir: "_site",
serveStaticOptions: {
extensions: ["html"]
}
}
});


return {
passthroughFileCopy: true,
Expand Down
3 changes: 2 additions & 1 deletion docs/package.json
View file
Open in desktop
Expand Up @@ -14,7 +14,7 @@
"watch:eleventy": "eleventy --serve --port=2023",
"build:sass": "sass --style=compressed src/assets/scss:src/assets/css --no-source-map",
"build:eleventy": "npx @11ty/eleventy",
"start": "npm-run-all build:sass --parallel watch:* --parallel images",
"start": "cross-env CONTEXT=local-serve npm-run-all build:sass --parallel watch:* --parallel images",
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned earlier, let's leave this as-is.

"build": "npm-run-all build:sass --parallel build:* --parallel images"
},
"devDependencies": {
Expand All @@ -24,6 +24,7 @@
"@11ty/eleventy-plugin-rss": "^1.1.1",
"@11ty/eleventy-plugin-syntaxhighlight": "^3.1.2",
"algoliasearch": "^4.12.1",
"cross-env": "^7.0.3",
"dom-parser": "^0.1.6",
"eleventy-plugin-nesting-toc": "^1.3.0",
"eleventy-plugin-page-assets": "^0.3.0",
Expand Down
2 changes: 1 addition & 1 deletion docs/src/_includes/components/docs-index.html
View file
Open in desktop
@@ -1,7 +1,7 @@
{% set navPages = collections.docs | eleventyNavigation %}
{% macro renderNavListItem(entry) -%}
<li class="docs-index__item" {% if entry.children.length %}data-has-children {% endif %}>
<a href="{{ entry.url | url }}" {% if entry.url == page.url %} aria-current="true" {% endif %}>{{ entry.title }}</a>
<a href="{{ entry.url | url | prettyURL }}" {% if entry.url == page.url %} aria-current="true" {% endif %}>{{ entry.title }}</a>
{%- if entry.children.length -%}
<ul data-child-list>
{%- for child in entry.children %}{{ renderNavListItem(child) }}{% endfor -%}
Expand Down
4 changes: 2 additions & 2 deletions docs/src/_includes/layouts/base.njk
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@
<!-- SEO -->
<title>{{ title }}</title>
<meta name="description" content="{{ desc }}">
<!--<link rel="canonical" href="https://docs.eslint.org{{ page.url | url }}">-->
<!--<link rel="canonical" href="https://docs.eslint.org{{ page.url | url | prettyURL }}">-->

<!-- favicon -->
<link rel="icon" href="/favicon.ico" sizes="any"><!-- 32×32 -->
Expand All @@ -25,7 +25,7 @@
<meta property="og:image:alt" content="{{ coverAlt }}">
<meta property="og:locale" content="en_US">
<meta property="og:type" content="website">
<meta property="og:url" content="{{ metadata.url }}{{ page.url }}">
<meta property="og:url" content="{{ metadata.url }}{{ page.url | url | prettyURL }}">
<!-- Twitter -->
<meta name="twitter:title" content="{{ title }}">
<meta name="twitter:card" content="summary_large_image">
Expand Down
2 changes: 1 addition & 1 deletion docs/src/_includes/layouts/components.html
View file
Open in desktop
Expand Up @@ -27,7 +27,7 @@
<ul class="index__list" id="js-index-list">
{% for item in collections.library %}
<li class="index__item">
<a href="{{ item.url | url }}" {{ helpers.getLinkActiveState(item.url,
<a href="{{ item.url | url | prettyURL }}" {{ helpers.getLinkActiveState(item.url,
page.url) | safe }}>{{ item.data.title }}</a>
</li>
{% endfor %}
Expand Down
2 changes: 1 addition & 1 deletion docs/src/library/library.json
View file
Open in desktop
@@ -1,4 +1,4 @@
{
"layout": "components.html",
"permalink": "/component-library/{{ page.fileSlug }}/index.html"
"permalink": "/component-library/{{ page.fileSlug }}.html"
}
3 changes: 3 additions & 0 deletions docs/src/src.json
View file
Open in desktop
@@ -0,0 +1,3 @@
{
"permalink": "{{ page.filePathStem }}.html"
}
2 changes: 1 addition & 1 deletion docs/src/static/sitemap.njk
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@ eleventyExcludeFromCollections: true
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
{% for page in collections.all %}
<url>
<loc>{{ site.url }}{{ page.url | url }}</loc>
<loc>{{ site.url }}{{ page.url | url | prettyURL }}</loc>
<lastmod>{{ page.date.toISOString() }}</lastmod>
<changefreq>{{ page.data.changeFreq or &quot;monthly&quot; }}</changefreq>
</url>
Expand Down