fix(pretty-urls): Limiting pretty URLs for trailing index URL segment

For the sake of more wide deployment support, we limit/restrict the pretty URL support for trailing "index" URL segments only. Before it was not restricted to this, means: "foo.com/mypage.html" was transformed "foo.com/mypage". This case is taken out now and will stay like "foo.com/mypage.html". Fixes #224
andi1984 · Dec 7, 2020 · 710b430 · 710b430
1 parent 0fdd3f1
commit 710b430
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 54 deletions.
diff --git a/README.md b/README.md
@@ -40,16 +40,16 @@ The output files are saved to the `dist` directory.
 
 ## CLI Options
 
-| Option   | Alias |                                                                                                  Description | Default |
-| -------- | :---: | -----------------------------------------------------------------------------------------------------------: | ------- |
-| --pretty |  --p  | Flag to indicate whether hosting can handle pretty URLs (e.g. `/hello/world` instead of `/hello/world.html`) | false   |
-| --clean  |  --c  |                           Flag to indicate whether distribution folder should be cleaned before every build. | false   |
+| Option  | Alias |                                                                        Description | Default |
+| ------- | :---: | ---------------------------------------------------------------------------------: | ------- |
+| --clean |  --c  | Flag to indicate whether distribution folder should be cleaned before every build. | false   |
 
 ## Environment Variables
 
-| Variable |   Description    |                      Example |
-| -------- | :--------------: | ---------------------------: |
-| URL      | URL of your page | `URL: https://www.google.de` |
+| Variable |                                                Description                                                 |                      Example |
+| -------- | :--------------------------------------------------------------------------------------------------------: | ---------------------------: |
+| URL      |                                              URL of your page                                              | `URL: https://www.google.de` |
+| PRETTY   | indicate whether hosting can handle pretty URLs (e.g. `/hello/world` instead of `/hello/world/index.html`) |               `PRETTY: true` |
 
 ## Configuration
 

diff --git a/utils/__mocks__/args.ts b/utils/__mocks__/args.ts
@@ -1 +1 @@
-module.exports = { _: [], p: true, pretty: true };
+module.exports = { _: [] };
diff --git a/utils/__tests__/url.test.ts b/utils/__tests__/url.test.ts
@@ -1,44 +1,73 @@
-const { isAbsoluteURL, generateURL } = require('../url');
+const { isAbsoluteURL, generateURL } = require("../url");
 
-jest.mock('../args');
+jest.mock("../args");
 
-describe('isAbsoluteURL', () => {
-  test('it fails if url is not given', () => {
+describe("isAbsoluteURL", () => {
+  test("it fails if url is not given", () => {
     expect(() => isAbsoluteURL()).toThrow();
   });
 
-  test('it returns true if url is absolute', () => {
-    expect(isAbsoluteURL('https://www.google.com')).toBe(true);
+  test("it returns true if url is absolute", () => {
+    expect(isAbsoluteURL("https://www.google.com")).toBe(true);
   });
 
-  test('it returns false if url is relative', () => {
-    expect(isAbsoluteURL('../foo')).toBe(false);
+  test("it returns false if url is relative", () => {
+    expect(isAbsoluteURL("../foo")).toBe(false);
   });
 });
 
+describe("generateURL", () => {
+  // Restore original env vars
+  // NOTE: https://stackoverflow.com/a/48042799/778340
+  const OLD_ENV = process.env;
 
-describe('generateURL', () => {
-    test('it fails if url is not given or not valid', () => {
-        expect(() => generateURL()).toThrow();
-    })
+  beforeEach(() => {
+    jest.resetModules(); // most important - it clears the cache
+    process.env = { ...OLD_ENV }; // make a copy
+  });
 
-    test('pathname', () => {
-        expect(generateURL('http://www.google.de').pathname).toBe('/');
-        expect(generateURL('http://www.google.de/foo').pathname).toBe('/foo');
-    })
+  afterAll(() => {
+    process.env = OLD_ENV; // restore old env
+  });
 
-    test('href', () => {
-        expect(generateURL('http://www.google.de').href).toBe(
-          'http://www.google.de/'
-        );
+  test("it fails if url is not given or not valid", () => {
+    expect(() => generateURL()).toThrow();
+  });
 
-        expect(generateURL('http://www.google.de/foo?myquery=2').href).toBe(
-          'http://www.google.de/foo?myquery=2'
-        );
-    })
+  test("pathname", () => {
+    expect(generateURL("http://www.google.de").pathname).toBe("/");
+    expect(generateURL("http://www.google.de/foo").pathname).toBe("/foo");
+  });
+
+  test("href", () => {
+    expect(generateURL("http://www.google.de").href).toBe(
+      "http://www.google.de/"
+    );
+
+    expect(generateURL("http://www.google.de/foo?myquery=2").href).toBe(
+      "http://www.google.de/foo?myquery=2"
+    );
+  });
 
-    test('Removes /index on prettyfied mode', () => {
-      process.env.URL = 'http://foo.de';
-      expect(generateURL('subfolder/index').href).toBe('http://foo.de/subfolder')
-    })
-})
+  test("Removes /index on pretty mode", () => {
+    process.env.PRETTY = "true";
+    process.env.URL = "http://foo.de";
+    expect(generateURL("subfolder/index").href).toBe("http://foo.de/subfolder");
+  });
+
+  test("Does not remove /index.html on non-pretty mode", () => {
+    process.env.PRETTY = "false";
+    process.env.URL = "http://foo.de";
+    expect(generateURL("subfolder/index").href).toBe(
+      "http://foo.de/subfolder/index.html"
+    );
+  });
+
+  test("Does not remove trailing segments other than index on pretty mode", () => {
+    process.env.PRETTY = "true";
+    process.env.URL = "http://foo.de";
+    expect(generateURL("subfolder/impressum").href).toBe(
+      "http://foo.de/subfolder/impressum.html"
+    );
+  });
+});
diff --git a/utils/args.ts b/utils/args.ts
@@ -3,14 +3,8 @@ export {};
 const yargs = require("yargs");
 
 // Exports all CLI arguments
-module.exports = yargs
-  .option("clean", {
-    alias: "c",
-    describe: "Cleanup dist folder on startup",
-    type: "boolean",
-  })
-  .option("pretty", {
-    alias: "p",
-    describe: "Prettify URLs",
-    type: "boolean",
-  }).argv;
+module.exports = yargs.option("clean", {
+  alias: "c",
+  describe: "Cleanup dist folder on startup",
+  type: "boolean",
+}).argv;
diff --git a/utils/url.ts b/utils/url.ts
@@ -19,12 +19,16 @@ const isAbsoluteURL = (url: string): Boolean => {
  * @returns {URL}
  */
 const generateURL = (link: string): URL => {
-  return new URL(
-    Boolean(args.pretty) || isAbsoluteURL(link) || link.indexOf(".html") !== -1
-      ? link.replace(/\/index$/g, "")
-      : `${link}.html`,
-    process.env.URL
-  );
+  if (isAbsoluteURL(link)) {
+    return new URL(link);
+  }
+
+  if (process.env.PRETTY === "true" && link.endsWith("index")) {
+    // In case we have support for pretty urls and link ends with index, we omit the last URL segment
+    return new URL(link.replace(/\/index$/g, ""), process.env.URL);
+  }
+
+  return new URL(`${link}.html`, process.env.URL);
 };
 
 module.exports = {