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

links in index.html does not contain .html suffix, but generated html files has #45

Closed
dawncold opened this issue Jul 19, 2021 · 5 comments · Fixed by #84
Closed

links in index.html does not contain .html suffix, but generated html files has #45

dawncold opened this issue Jul 19, 2021 · 5 comments · Fixed by #84
Labels
bug Something isn't working

Comments

@dawncold
Copy link

Bug Report

Description

links in index.html does not contain .html suffix, but generated html files has.

Steps to Reproduce

log4brains build -o public

Expected Behavior

should generated links with .html suffix

Context

Environment

  • Log4brains version: (v1.0.0-beta.11)
  • Node.js version:
  • OS and its version:
  • Browser information: Chrome latest

Possible Solution

found this question: https://stackoverflow.com/questions/62867105/how-to-deal-with-nextjs-exporting-files-with-html-extension-but-inlink-there

maybe we should add this for Next framework.

@dawncold dawncold added the bug Something isn't working label Jul 19, 2021
@deviantintegral
Copy link

At a basic level, setting trailingSlash: true, fixes the issue for me. As noted in the stack overflow threads, it does mean ADRs are rendered as directories, such as http://localhost/adr/0001-record-architecture-decisions/. However, that could actually be a good thing if we get inline images working, because then those assets could be included in the ADR path.

@fire
Copy link

fire commented Jan 9, 2022

How do you enable that?

@deviantintegral

@deviantintegral
Copy link

You can set it in next.config.js: https://nextjs.org/docs/api-reference/next.config.js/trailing-slash

@llwt
Copy link
Contributor

llwt commented Sep 21, 2022

For anyone coming to this issue looking to apply this fix, you can use patch-package while we wait for this change to be merged:

The autogenerated output from patch-package is below :)


Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch @log4brains/[email protected] for the project I'm working on.

Here is the diff that solved my problem:

diff --git a/node_modules/@log4brains/web/dist/next.config.js b/node_modules/@log4brains/web/dist/next.config.js
index a5dd2e9..4d14fab 100644
--- a/node_modules/@log4brains/web/dist/next.config.js
+++ b/node_modules/@log4brains/web/dist/next.config.js
@@ -13,6 +13,16 @@ module.exports = withBundleAnalyzer({
   reactStrictMode: true,
   target: "serverless",
   poweredByHeader: false,
+  /**
+   * This patch is to ensure that next outputs page/index.html instead of page.html
+   * Without this, links to pages other than the index file will not work on s3
+   * 
+   * without:
+   *   .log4brains/out/adr/2022022-document-frontend-infra-decisions.html
+   * with:
+   *   .log4brains/out/adr/2022022-document-frontend-infra-decisions/index.html
+   */
+  trailingSlash: true,
   serverRuntimeConfig: {
     PROJECT_ROOT: __dirname, // https://github.com/vercel/next.js/issues/8251
     VERSION: process.env.HIDE_LOG4BRAINS_VERSION ? "" : packageJson.version

This issue body was partially generated by patch-package.

llwt added a commit to llwt/log4brains that referenced this issue Sep 21, 2022
thomvaill pushed a commit that referenced this issue Sep 22, 2022
ADRs are now generated to adr/<name>/index.html instead of adr/<name>.html. See #45 for context.
@thomvaill
Copy link
Owner

LGTM. Thank you all

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
Development

Successfully merging a pull request may close this issue.

5 participants