fix: fix Locale Dropdown RTL icon + Webpack aliases ordering (#5383)

* fix Locale dropdown IconLanguage RTL margin

* add more alias test fixtures for nested elements

* change webpack alias ordering logic to handle nested items better

* another aliases order fix

packages/docusaurus-theme-classic/src/theme/NavbarItem/LocaleDropdownNavbarItem/index.tsx

@@ -13,6 +13,8 @@ import useDocusaurusContext from '@docusaurus/useDocusaurusContext';
 import {useAlternatePageUtils} from '@docusaurus/theme-common';
 import type {LinkLikeNavbarItemProps} from '@theme/NavbarItem';
 
+import styles from './styles.module.css';
+
 export default function LocaleDropdownNavbarItem({
   mobile,
   dropdownItemsBefore,
@@ -58,9 +60,7 @@ export default function LocaleDropdownNavbarItem({
       mobile={mobile}
       label={
         <span>
-          <IconLanguage
-            style={{verticalAlign: 'text-bottom', marginRight: 5}}
-          />
+          <IconLanguage className={styles.iconLanguage} />
           <span>{dropdownLabel}</span>
         </span>
       }

packages/docusaurus-theme-classic/src/theme/NavbarItem/LocaleDropdownNavbarItem/styles.module.css

@@ -0,0 +1,11 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+.iconLanguage {
+  vertical-align: text-bottom;
+  margin-right: 5px;
+}

packages/docusaurus/src/server/themes/__tests__/alias.test.ts

@@ -14,12 +14,15 @@ describe('themeAlias', () => {
     const fixtures = path.join(__dirname, '__fixtures__');
     const themePath = path.join(fixtures, 'theme-1');
     const alias = themeAlias(themePath, true);
-    expect(alias).toEqual({
-      '@theme/Footer': path.join(themePath, 'Footer/index.js'),
-      '@theme-original/Footer': path.join(themePath, 'Footer/index.js'),
-      '@theme/Layout': path.join(themePath, 'Layout.js'),
-      '@theme-original/Layout': path.join(themePath, 'Layout.js'),
-    });
+    // Testing entries, because order matters!
+    expect(Object.entries(alias)).toEqual(
+      Object.entries({
+        '@theme-original/Footer': path.join(themePath, 'Footer/index.js'),
+        '@theme-original/Layout': path.join(themePath, 'Layout.js'),
+        '@theme/Footer': path.join(themePath, 'Footer/index.js'),
+        '@theme/Layout': path.join(themePath, 'Layout.js'),
+      }),
+    );
     expect(alias).not.toEqual({});
   });
 
@@ -27,10 +30,13 @@ describe('themeAlias', () => {
     const fixtures = path.join(__dirname, '__fixtures__');
     const themePath = path.join(fixtures, 'theme-1');
     const alias = themeAlias(themePath, false);
-    expect(alias).toEqual({
-      '@theme/Footer': path.join(themePath, 'Footer/index.js'),
-      '@theme/Layout': path.join(themePath, 'Layout.js'),
-    });
+    // Testing entries, because order matters!
+    expect(Object.entries(alias)).toEqual(
+      Object.entries({
+        '@theme/Footer': path.join(themePath, 'Footer/index.js'),
+        '@theme/Layout': path.join(themePath, 'Layout.js'),
+      }),
+    );
     expect(alias).not.toEqual({});
   });
 
@@ -38,12 +44,42 @@ describe('themeAlias', () => {
     const fixtures = path.join(__dirname, '__fixtures__');
     const themePath = path.join(fixtures, 'theme-2');
     const alias = themeAlias(themePath, true);
-    expect(alias).toEqual({
-      '@theme/Navbar': path.join(themePath, 'Navbar.js'),
-      '@theme-original/Navbar': path.join(themePath, 'Navbar.js'),
-      '@theme/Layout': path.join(themePath, 'Layout/index.js'),
-      '@theme-original/Layout': path.join(themePath, 'Layout/index.js'),
-    });
+    // Testing entries, because order matters!
+    expect(Object.entries(alias)).toEqual(
+      Object.entries({
+        '@theme-original/Layout': path.join(themePath, 'Layout/index.js'),
+        '@theme-original/Navbar': path.join(themePath, 'Navbar.js'),
+        '@theme-original/NavbarItem/NestedNavbarItem': path.join(
+          themePath,
+          'NavbarItem/NestedNavbarItem/index.js',
+        ),
+        '@theme-original/NavbarItem/SiblingNavbarItem': path.join(
+          themePath,
+          'NavbarItem/SiblingNavbarItem.js',
+        ),
+        '@theme-original/NavbarItem/zzz': path.join(
+          themePath,
+          'NavbarItem/zzz.js',
+        ),
+        '@theme-original/NavbarItem': path.join(
+          themePath,
+          'NavbarItem/index.js',
+        ),
+
+        '@theme/Layout': path.join(themePath, 'Layout/index.js'),
+        '@theme/Navbar': path.join(themePath, 'Navbar.js'),
+        '@theme/NavbarItem/NestedNavbarItem': path.join(
+          themePath,
+          'NavbarItem/NestedNavbarItem/index.js',
+        ),
+        '@theme/NavbarItem/SiblingNavbarItem': path.join(
+          themePath,
+          'NavbarItem/SiblingNavbarItem.js',
+        ),
+        '@theme/NavbarItem/zzz': path.join(themePath, 'NavbarItem/zzz.js'),
+        '@theme/NavbarItem': path.join(themePath, 'NavbarItem/index.js'),
+      }),
+    );
     expect(alias).not.toEqual({});
   });
 
@@ -51,10 +87,23 @@ describe('themeAlias', () => {
     const fixtures = path.join(__dirname, '__fixtures__');
     const themePath = path.join(fixtures, 'theme-2');
     const alias = themeAlias(themePath, false);
-    expect(alias).toEqual({
-      '@theme/Navbar': path.join(themePath, 'Navbar.js'),
-      '@theme/Layout': path.join(themePath, 'Layout/index.js'),
-    });
+    // Testing entries, because order matters!
+    expect(Object.entries(alias)).toEqual(
+      Object.entries({
+        '@theme/Layout': path.join(themePath, 'Layout/index.js'),
+        '@theme/Navbar': path.join(themePath, 'Navbar.js'),
+        '@theme/NavbarItem/NestedNavbarItem': path.join(
+          themePath,
+          'NavbarItem/NestedNavbarItem/index.js',
+        ),
+        '@theme/NavbarItem/SiblingNavbarItem': path.join(
+          themePath,
+          'NavbarItem/SiblingNavbarItem.js',
+        ),
+        '@theme/NavbarItem/zzz': path.join(themePath, 'NavbarItem/zzz.js'),
+        '@theme/NavbarItem': path.join(themePath, 'NavbarItem/index.js'),
+      }),
+    );
     expect(alias).not.toEqual({});
   });
 

packages/docusaurus/src/server/themes/__tests__/index.test.ts

@@ -0,0 +1,61 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import path from 'path';
+import {loadThemeAliases} from '../index';
+
+describe('loadThemeAliases', () => {
+  test('next alias can override the previous alias', () => {
+    const fixtures = path.join(__dirname, '__fixtures__');
+    const theme1Path = path.join(fixtures, 'theme-1');
+    const theme2Path = path.join(fixtures, 'theme-2');
+
+    const alias = loadThemeAliases([theme1Path, theme2Path]);
+
+    // Testing entries, because order matters!
+    expect(Object.entries(alias)).toEqual(
+      Object.entries({
+        '@theme-init/Layout': path.join(theme1Path, 'Layout.js'),
+
+        '@theme-original/Footer': path.join(theme1Path, 'Footer/index.js'),
+        '@theme-original/Layout': path.join(theme2Path, 'Layout/index.js'),
+        '@theme-original/Navbar': path.join(theme2Path, 'Navbar.js'),
+        '@theme-original/NavbarItem/NestedNavbarItem': path.join(
+          theme2Path,
+          'NavbarItem/NestedNavbarItem/index.js',
+        ),
+        '@theme-original/NavbarItem/SiblingNavbarItem': path.join(
+          theme2Path,
+          'NavbarItem/SiblingNavbarItem.js',
+        ),
+        '@theme-original/NavbarItem/zzz': path.join(
+          theme2Path,
+          'NavbarItem/zzz.js',
+        ),
+        '@theme-original/NavbarItem': path.join(
+          theme2Path,
+          'NavbarItem/index.js',
+        ),
+
+        '@theme/Footer': path.join(theme1Path, 'Footer/index.js'),
+        '@theme/Layout': path.join(theme2Path, 'Layout/index.js'),
+        '@theme/Navbar': path.join(theme2Path, 'Navbar.js'),
+        '@theme/NavbarItem/NestedNavbarItem': path.join(
+          theme2Path,
+          'NavbarItem/NestedNavbarItem/index.js',
+        ),
+        '@theme/NavbarItem/SiblingNavbarItem': path.join(
+          theme2Path,
+          'NavbarItem/SiblingNavbarItem.js',
+        ),
+        '@theme/NavbarItem/zzz': path.join(theme2Path, 'NavbarItem/zzz.js'),
+        '@theme/NavbarItem': path.join(theme2Path, 'NavbarItem/index.js'),
+      }),
+    );
+    expect(alias).not.toEqual({});
+  });
+});

packages/docusaurus/src/server/themes/__tests__/index.ts

@@ -1,29 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- */
-
-import path from 'path';
-import {loadThemeAliases} from '../index';
-
-describe('loadThemeAliases', () => {
-  test('next alias can override the previous alias', () => {
-    const fixtures = path.join(__dirname, '__fixtures__');
-    const theme1Path = path.join(fixtures, 'theme-1');
-    const theme2Path = path.join(fixtures, 'theme-2');
-
-    const alias = loadThemeAliases([theme1Path, theme2Path]);
-    expect(alias).toEqual({
-      '@theme-init/Layout': path.join(theme1Path, 'Layout.js'), // TODO: Write separate test case for this?
-      '@theme/Footer': path.join(theme1Path, 'Footer/index.js'),
-      '@theme-original/Footer': path.join(theme1Path, 'Footer/index.js'),
-      '@theme/Navbar': path.join(theme2Path, 'Navbar.js'),
-      '@theme-original/Navbar': path.join(theme2Path, 'Navbar.js'),
-      '@theme/Layout': path.join(theme2Path, 'Layout/index.js'),
-      '@theme-original/Layout': path.join(theme2Path, 'Layout/index.js'),
-    });
-    expect(alias).not.toEqual({});
-  });
-});

packages/docusaurus/src/server/themes/alias.ts

@@ -11,6 +11,20 @@ import {fileToPath, posixPath, normalizeUrl, Globby} from '@docusaurus/utils';
 import {ThemeAliases} from '@docusaurus/types';
 import {sortBy} from 'lodash';
 
+// Order of Webpack aliases is important because one alias can shadow another
+// This ensure @theme/NavbarItem alias is after @theme/NavbarItem/LocaleDropdown
+// See https://github.com/facebook/docusaurus/pull/3922
+// See https://github.com/facebook/docusaurus/issues/5382
+export function sortAliases(aliases: ThemeAliases): ThemeAliases {
+  // Alphabetical order by default
+  const entries = sortBy(Object.entries(aliases), ([alias]) => alias);
+  // @theme/NavbarItem should be after @theme/NavbarItem/LocaleDropdown
+  entries.sort(([alias1], [alias2]) => {
+    return alias1.includes(`${alias2}/`) ? -1 : 0;
+  });
+  return Object.fromEntries(entries);
+}
+
 // TODO make async
 export default function themeAlias(
   themePath: string,
@@ -24,15 +38,9 @@ export default function themeAlias(
     cwd: themePath,
   });
 
-  // See https://github.com/facebook/docusaurus/pull/3922
-  // ensure @theme/NavbarItem alias is created after @theme/NavbarItem/LocaleDropdown
-  const sortedThemeComponentFiles = sortBy(themeComponentFiles, (file) =>
-    file.endsWith('/index.js'),
-  );
-
   const aliases: ThemeAliases = {};
 
-  sortedThemeComponentFiles.forEach((relativeSource) => {
+  themeComponentFiles.forEach((relativeSource) => {
     const filePath = path.join(themePath, relativeSource);
     const fileName = fileToPath(relativeSource);
 
@@ -50,5 +58,5 @@ export default function themeAlias(
     }
   });
 
-  return aliases;
+  return sortAliases(aliases);
 }

packages/docusaurus/src/server/themes/index.ts

@@ -8,7 +8,7 @@
 import {ThemeAliases, LoadedPlugin} from '@docusaurus/types';
 import path from 'path';
 import {THEME_PATH} from '../../constants';
-import themeAlias from './alias';
+import themeAlias, {sortAliases} from './alias';
 
 const ThemeFallbackDir = path.resolve(__dirname, '../../client/theme-fallback');
 
@@ -44,7 +44,7 @@ export function loadThemeAliases(
     aliases = {...aliases, ...buildThemeAliases(userThemeAliases, aliases)};
   });
 
-  return aliases;
+  return sortAliases(aliases);
 }
 
 export function loadPluginsThemeAliases({