Introduce a new URL for library pages, at index.html

Fixes dart-lang#1346 This "moves" the URL of each library from, e.g. 'package-two_lib2/package-two_lib2-library.html' to 'package-two_lib2/index.html', such that we can have links with 'package-two_lib2'. The old URLs are preserved via redirecting HTML files. (HTTP redirects are more standard and recommended, but dartdoc does not contain an HTTP server; HTTP redirects could be implemented at the hosting server, which is different for different doc hosts (Google Storage, Firebase, ...). Instead we use simpler HTML redirects. In fact, a given backend host could use the HTML redirecting files as a database to inform HTTP redirect rules, that need to be implemented differently for each backend. So dartdoc now produces redirecting files, but a given backend could instead return an HTTP 301 Permanent Redirect response when requesting one of the redirecting files.) This adds a negligible number of files (1 per documented library), and a negligible number of bytes (tiny redirect files), but actually is probably a net negative in terms of bytes, as many links are now shortened, removing the HTML file name of library URLs.
srawlins · Oct 2, 2024 · fa212e1 · fa212e1
1 parent 76678f6
commit fa212e1
Show file tree

Hide file tree

Showing 14 changed files with 245 additions and 46 deletions.
diff --git a/lib/src/generator/generator_backend.dart b/lib/src/generator/generator_backend.dart
@@ -179,11 +179,14 @@ abstract class GeneratorBackend {
     runtimeStats.incrementAccumulator('writtenFunctionFileCount');
   }
 
-  /// Emits documentation content for the [library].
+  /// Emits documentation content for the [library], and the content for the
+  /// library's previous location (which just redirects to the new location).
   void generateLibrary(PackageGraph packageGraph, Library library) {
     var data = LibraryTemplateData(options, packageGraph, library);
     var content = templates.renderLibrary(data);
+    var redirectContent = templates.renderLibraryRedirect(data);
     write(writer, library.filePath, data, content);
+    write(writer, library.redirectingPath, data, redirectContent);
     runtimeStats.incrementAccumulator('writtenLibraryFileCount');
   }
 

diff --git a/lib/src/generator/templates.aot_renderers_for_html.dart b/lib/src/generator/templates.aot_renderers_for_html.dart
@@ -1137,6 +1137,29 @@ String renderLibrary(LibraryTemplateData context0) {
   return buffer.toString();
 }
 
+String renderLibraryRedirect(LibraryTemplateData context0) {
+  final buffer = StringBuffer();
+  buffer.write('''<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta http-equiv="refresh" content="0; url=''');
+  if (context0.useBaseHref) {
+    var context1 = context0.htmlBase;
+    buffer.write(context0.htmlBase);
+    buffer.writeEscaped(context0.self.href);
+  }
+  buffer.write('''" />
+  </head>
+  <body>
+    <p><a href="''');
+  buffer.writeEscaped(context0.self.href);
+  buffer.write('''">Redirect</a></p>
+  </body>
+</html>''');
+
+  return buffer.toString();
+}
+
 String renderMethod(MethodTemplateData context0) {
   final buffer = StringBuffer();
   buffer.write(_renderMethod_partial_head_0(context0));

diff --git a/lib/src/generator/templates.dart b/lib/src/generator/templates.dart
@@ -22,6 +22,8 @@
 @Renderer(#renderFunction, Context<FunctionTemplateData>(), 'function')
 @Renderer(#renderIndex, Context<PackageTemplateData>(), 'index')
 @Renderer(#renderLibrary, Context<LibraryTemplateData>(), 'library')
+@Renderer(
+    #renderLibraryRedirect, Context<LibraryTemplateData>(), 'library_redirect')
 @Renderer(#renderMethod, Context<MethodTemplateData>(), 'method')
 @Renderer(#renderMixin, Context<MixinTemplateData>(), 'mixin')
 @Renderer(#renderProperty, Context<PropertyTemplateData>(), 'property')
@@ -99,6 +101,7 @@ abstract class Templates {
   String renderFunction(FunctionTemplateData context);
   String renderIndex(PackageTemplateData context);
   String renderLibrary(LibraryTemplateData context);
+  String renderLibraryRedirect(LibraryTemplateData context);
   String renderMethod(MethodTemplateData context);
   String renderMixin(MixinTemplateData context);
   String renderProperty(PropertyTemplateData context);
@@ -174,6 +177,10 @@ class HtmlAotTemplates implements Templates {
   String renderLibrary(LibraryTemplateData context) =>
       aot_renderers_for_html.renderLibrary(context);
 
+  @override
+  String renderLibraryRedirect(LibraryTemplateData context) =>
+      aot_renderers_for_html.renderLibraryRedirect(context);
+
   @override
   String renderMethod(MethodTemplateData context) =>
       aot_renderers_for_html.renderMethod(context);
@@ -251,6 +258,10 @@ class RuntimeTemplates implements Templates {
   String renderLibrary(LibraryTemplateData context) =>
       runtime_renderers.renderLibrary(context, _libraryTemplate);
 
+  @override
+  String renderLibraryRedirect(LibraryTemplateData context) =>
+      runtime_renderers.renderLibraryRedirect(context, _libraryTemplate);
+
   @override
   String renderMethod(MethodTemplateData context) =>
       runtime_renderers.renderMethod(context, _methodTemplate);

diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart
@@ -8443,6 +8443,28 @@ class _Renderer_Library extends RendererBase<Library> {
                         parent: r));
                   },
                 ),
+                'redirectingPath': Property(
+                  getValue: (CT_ c) => c.redirectingPath,
+                  renderVariable:
+                      (CT_ c, Property<CT_> self, List<String> remainingNames) {
+                    if (remainingNames.isEmpty) {
+                      return self.getValue(c).toString();
+                    }
+                    var name = remainingNames.first;
+                    var nextProperty =
+                        _Renderer_String.propertyMap().getValue(name);
+                    return nextProperty.renderVariable(
+                        self.getValue(c) as String,
+                        nextProperty,
+                        [...remainingNames.skip(1)]);
+                  },
+                  isNullValue: (CT_ c) => false,
+                  renderValue: (CT_ c, RendererBase<CT_> r,
+                      List<MustachioNode> ast, StringSink sink) {
+                    _render_String(c.redirectingPath, ast, r.template, sink,
+                        parent: r);
+                  },
+                ),
                 'referenceChildren': Property(
                   getValue: (CT_ c) => c.referenceChildren,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -8808,6 +8830,12 @@ class _Renderer_LibraryTemplateData extends RendererBase<LibraryTemplateData> {
   }
 }
 
+String renderLibraryRedirect(LibraryTemplateData context, Template template) {
+  var buffer = StringBuffer();
+  _render_LibraryTemplateData(context, template.ast, template, buffer);
+  return buffer.toString();
+}
+
 class _Renderer_Locatable extends RendererBase<Locatable> {
   static final Map<Type, Object> _propertyMapCache = {};
   static Map<String, Property<CT_>> propertyMap<CT_ extends Locatable>() =>
@@ -12482,13 +12510,13 @@ class _Renderer_PackageTemplateData extends RendererBase<PackageTemplateData> {
   }
 }
 
-String renderIndex(PackageTemplateData context, Template template) {
+String renderError(PackageTemplateData context, Template template) {
   var buffer = StringBuffer();
   _render_PackageTemplateData(context, template.ast, template, buffer);
   return buffer.toString();
 }
 
-String renderError(PackageTemplateData context, Template template) {
+String renderIndex(PackageTemplateData context, Template template) {
   var buffer = StringBuffer();
   _render_PackageTemplateData(context, template.ast, template, buffer);
   return buffer.toString();

diff --git a/lib/src/model/library.dart b/lib/src/model/library.dart
@@ -199,7 +199,7 @@ class Library extends ModelElement
   String get filePath => '$dirName/$fileName';
 
   @override
-  String get fileName => '$dirName-library.html';
+  String get fileName => 'index.html';
 
   String get sidebarPath => '$dirName/$dirName-library-sidebar.html';
 
@@ -216,9 +216,20 @@ class Library extends ModelElement
     if (!identical(canonicalModelElement, this)) {
       return canonicalModelElement?.href;
     }
-    return '${package.baseHref}$filePath';
+    // The file name for a library is 'index.html', so we just link to the
+    // directory name. This keeps the URL looking short, _without_ the
+    // 'index.html' in the URL.
+    return '${package.baseHref}$dirName';
   }
 
+  /// The previous value of [filePath].
+  ///
+  /// This path is used to write a file that ontains an HTML redirect (not an
+  /// HTTP redirect) to a library's current [filePath].
+  String get redirectingPath => '$dirName/$dirName-library.html';
+
+  /// Whether a libary is anonymous, either because it has no library directive
+  /// or it has a library directive without a name.
   bool get isAnonymous => element.name.isEmpty;
 
   @override

diff --git a/lib/src/model/package.dart b/lib/src/model/package.dart
@@ -136,9 +136,13 @@ class Package extends LibraryContainer
   bool get isPublic =>
       _isLocalPublicByDefault || libraries.any((l) => l.isPublic);
 
-  /// Return true if this is the default package, this is part of an embedder
-  /// SDK, or if [DartdocOptionContext.autoIncludeDependencies] is true -- but
-  /// only if the package was not excluded on the command line.
+  /// Whether this package is local.
+  ///
+  /// A package can be local in three ways:
+  /// * this is the default package,
+  /// * this is part of an embedder SDK, or
+  /// * [DartdocOptionContext.autoIncludeDependencies] is true, and this
+  ///   package is not excluded with `exclude-packages`.
   bool get isLocal {
     // Do not document as local if we excluded this package by name.
     if (_isExcluded) return false;

diff --git a/lib/src/validator.dart b/lib/src/validator.dart
@@ -61,8 +61,7 @@ class Validator {
       return;
     }
     _visited.add(fullPath);
-    final links = pageLinks.links;
-    final baseHref = pageLinks.baseHref;
+    var (links, baseHref) = pageLinks;
 
     // Prevent extremely large stacks by storing the paths we are using
     // here instead -- occasionally, very large jobs have overflowed
@@ -75,10 +74,8 @@ class Validator {
     for (final href in links) {
       final uri = Uri.tryParse(href);
       if (uri == null || !uri.hasAuthority && !uri.hasFragment) {
-        var linkPath = '$pathDirectory/$href';
-
-        linkPath = path.normalize(linkPath);
-        final newFullPath = path.join(_origin, linkPath);
+        var linkPath = path.normalize(path.url.join(pathDirectory, href));
+        var newFullPath = path.join(_origin, linkPath);
         if (!_visited.contains(newFullPath)) {
           toVisit.add((linkPath, newFullPath));
           _visited.add(newFullPath);
@@ -156,13 +153,17 @@ class Validator {
     found.add(indexPath);
     for (var entry in jsonData.cast<Map<String, dynamic>>()) {
       if (entry.containsKey('href')) {
-        final entryPath = path
-            .joinAll([_origin, ...path.posix.split(entry['href'] as String)]);
-        if (!_visited.contains(entryPath)) {
-          _warn(PackageWarning.brokenLink, entryPath, _origin,
+        var href =
+            path.joinAll([_origin, ...path.url.split(entry['href'] as String)]);
+        if (path.extension(href).isEmpty) {
+          // An aliased link to an `index.html` file.
+          href = path.url.join(href, 'index.html');
+        }
+        if (!_visited.contains(href)) {
+          _warn(PackageWarning.brokenLink, href, _origin,
               referredFrom: fullPath);
         }
-        found.add(entryPath);
+        found.add(href);
       }
     }
     final missingFromSearch = _visited.difference(found);
@@ -177,7 +178,7 @@ class Validator {
   ///
   /// This is extracted to save memory during the check; be careful not to hang
   /// on to anything referencing the full file and doc tree.
-  _PageLinks? _getLinksAndBaseHref(String fullPath) {
+  (Set<String> links, String? baseHref)? _getLinksAndBaseHref(String fullPath) {
     final file = _config.resourceProvider.getFile(fullPath);
     if (!file.exists) {
       return null;
@@ -196,15 +197,20 @@ class Validator {
     final stringLinks = links
         .map((link) => link.attributes['href'])
         .nonNulls
-        .where((href) =>
-            href.isNotEmpty &&
-            !href.startsWith('https:') &&
-            !href.startsWith('http:') &&
-            !href.startsWith('mailto:') &&
-            !href.startsWith('ftp:'))
+        .where((uri) =>
+            uri.isNotEmpty &&
+            !uri.startsWith('https:') &&
+            !uri.startsWith('http:') &&
+            !uri.startsWith('mailto:') &&
+            !uri.startsWith('ftp:'))
+        .map((uri) =>
+            // An aliased link to an `index.html` file.
+            path.extension(uri).isEmpty
+                ? path.url.join(uri, 'index.html')
+                : uri)
         .toSet();
 
-    return _PageLinks(stringLinks, baseHref);
+    return (stringLinks, baseHref);
   }
 
   /// Warns on erroneous file paths.
@@ -246,11 +252,3 @@ class Validator {
         message: message, referredFrom: referredFromElements);
   }
 }
-
-/// Stores all links found in a page, and the base href.
-class _PageLinks {
-  final Set<String> links;
-  final String? baseHref;
-
-  const _PageLinks(this.links, this.baseHref);
-}
diff --git a/lib/templates/library_redirect.html b/lib/templates/library_redirect.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta http-equiv="refresh" content="0; url={{ #useBaseHref }}{{ #htmlBase }}{{{ htmlBase }}}{{ /htmlBase }}{{ self.href }}{{ /useBaseHref }}" />
+  </head>
+  <body>
+    <p><a href="{{ self.href }}">Redirect</a></p>
+  </body>
+</html>
diff --git a/test/dartdoc_test_base.dart b/test/dartdoc_test_base.dart
@@ -43,6 +43,11 @@ abstract class DartdocTestBase {
 
   String get sdkConstraint => '>=3.6.0 <4.0.0';
 
+  /// A mapping of pub dependency names to the paths.
+  ///
+  /// These are written out to a pubspec file using `path` keys.
+  Map<String, String> get pubDependencyPaths => const {};
+
   List<String> get experiments => ['enhanced-parts', 'wildcard-variables'];
 
   bool get skipUnreachableSdkLibraries => true;
@@ -61,7 +66,10 @@ abstract class DartdocTestBase {
   }
 
   Future<void> _setUpPackage() async {
-    var pubspec = d.buildPubspecText(sdkConstraint: sdkConstraint);
+    var pubspec = d.buildPubspecText(
+      sdkConstraint: sdkConstraint,
+      dependencies: pubDependencyPaths,
+    );
     String? analysisOptions;
     if (experiments.isNotEmpty) {
       analysisOptions = '''

diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart
@@ -1221,7 +1221,7 @@ void main() async {
         expect(
           aFunctionUsingRenamedLib.documentationAsHtml,
           contains(
-              'Link to library: <a href="${htmlBasePlaceholder}mylibpub/mylibpub-library.html">renamedLib</a>'),
+              'Link to library: <a href="${htmlBasePlaceholder}mylibpub">renamedLib</a>'),
         );
         expect(
           aFunctionUsingRenamedLib.documentationAsHtml,