Skip to content

Commit d858abc

Browse files
jdkorenjdkoren
authored
Remove <base> tag, use hrefs prepended with base (#2098)
* Remove <base> tag, use hrefs prepended with base * Add tests for relative hrefs in doc comments * Add a CLI flag (hidden) to use the legacy behavior * Detailed comment regarding usage of the placeholder * Fix placeholder string in header and footer partials
1 parent 7807ee1 commit d858abc

File tree

10 files changed

+244
-135
lines changed

10 files changed

+244
-135
lines changed

‎lib/src/dartdoc_options.dart‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,9 @@ class DartdocOptionContext extends DartdocOptionContextBase
14221422
excludePackages.any((pattern) => name == pattern);
14231423

14241424
String get templatesDir => optionSet['templatesDir'].valueAt(context);
1425+
1426+
// TODO(jdkoren): temporary while we confirm href base behavior doesn't break important clients
1427+
bool get useBaseHref => optionSet['useBaseHref'].valueAt(context);
14251428
}
14261429

14271430
/// Instantiate dartdoc's configuration file and options parser with the
@@ -1635,6 +1638,14 @@ Future<List<DartdocOption>> createDartdocOptions() async {
16351638
'property, top_level_constant, top_level_property, typedef. Partial templates are '
16361639
'supported; they must begin with an underscore, and references to them must omit the '
16371640
'leading underscore (e.g. use {{>foo}} to reference the partial template _foo.html).'),
1641+
DartdocOptionArgOnly<bool>('useBaseHref', false,
1642+
help:
1643+
'Use <base href> in generated files (legacy behavior). This option '
1644+
'is temporary and support will be removed in the future. Use only '
1645+
'if the default behavior breaks links between your documentation '
1646+
'pages, and please file an issue on Github.',
1647+
negatable: false,
1648+
hide: true),
16381649
// TODO(jcollins-g): refactor so there is a single static "create" for
16391650
// each DartdocOptionContext that traverses the inheritance tree itself.
16401651
]

‎lib/src/html/html_generator.dart‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,17 @@ class HtmlGeneratorOptions implements HtmlOptions {
142142
@override
143143
final String toolVersion;
144144

145+
@override
146+
final bool useBaseHref;
147+
145148
HtmlGeneratorOptions(
146149
{this.url,
147150
this.relCanonicalPrefix,
148151
this.faviconPath,
149152
String toolVersion,
150153
this.prettyIndexJson = false,
151-
this.templatesDir})
154+
this.templatesDir,
155+
this.useBaseHref = false})
152156
: this.toolVersion = toolVersion ?? 'unknown';
153157
}
154158

@@ -166,7 +170,8 @@ Future<List<Generator>> initGenerators(GeneratorContext config) async {
166170
toolVersion: dartdocVersion,
167171
faviconPath: config.favicon,
168172
prettyIndexJson: config.prettyIndexJson,
169-
templatesDir: config.templatesDir);
173+
templatesDir: config.templatesDir,
174+
useBaseHref: config.useBaseHref);
170175

171176
return [
172177
await HtmlGenerator.create(

‎lib/src/html/html_generator_instance.dart‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ class HtmlGeneratorInstance {
7676
});
7777

7878
String json = encoder.convert(indexItems);
79+
if (!_options.useBaseHref) {
80+
json = json.replaceAll(HTMLBASE_PLACEHOLDER, '');
81+
}
7982
_writer(path.join('categories.json'), '${json}\n');
8083
}
8184

@@ -118,6 +121,9 @@ class HtmlGeneratorInstance {
118121
});
119122

120123
String json = encoder.convert(indexItems);
124+
if (!_options.useBaseHref) {
125+
json = json.replaceAll(HTMLBASE_PLACEHOLDER, '');
126+
}
121127
_writer(path.join('index.json'), '${json}\n');
122128
}
123129

@@ -412,6 +418,10 @@ class HtmlGeneratorInstance {
412418
// Replaces '/' separators with proper separators for the platform.
413419
String outFile = path.joinAll(filename.split('/'));
414420
String content = template.renderString(data);
421+
422+
if (!_options.useBaseHref) {
423+
content = content.replaceAll(HTMLBASE_PLACEHOLDER, data.htmlBase);
424+
}
415425
_writer(outFile, content,
416426
element: data.self is Warnable ? data.self : null);
417427
if (data.self is Indexable) _indexedElements.add(data.self as Indexable);

‎lib/src/html/template_data.dart‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:dartdoc/src/model/model.dart';
88
abstract class HtmlOptions {
99
String get relCanonicalPrefix;
1010
String get toolVersion;
11+
bool get useBaseHref;
1112
}
1213

1314
abstract class TemplateData<T extends Documentable> {
@@ -38,6 +39,7 @@ abstract class TemplateData<T extends Documentable> {
3839
T get self;
3940
String get version => htmlOptions.toolVersion;
4041
String get relCanonicalPrefix => htmlOptions.relCanonicalPrefix;
42+
bool get useBaseHref => htmlOptions.useBaseHref;
4143

4244
String _layoutTitle(String name, String kind, bool isDeprecated) =>
4345
_renderHelper.composeLayoutTitle(name, kind, isDeprecated);
@@ -67,9 +69,9 @@ class PackageTemplateData extends TemplateData<Package> {
6769
bool get hasHomepage => package.hasHomepage;
6870
String get homepage => package.homepage;
6971

70-
/// `null` for packages because they are at the root – not needed
72+
/// empty for packages because they are at the root – not needed
7173
@override
72-
String get htmlBase => null;
74+
String get htmlBase => '';
7375
}
7476

7577
class CategoryTemplateData extends TemplateData<Category> {
@@ -83,7 +85,7 @@ class CategoryTemplateData extends TemplateData<Category> {
8385
String get title => '${category.name} ${category.kind} - Dart API';
8486

8587
@override
86-
String get htmlBase => '..';
88+
String get htmlBase => '../';
8789

8890
@override
8991
String get layoutTitle => _layoutTitle(category.name, category.kind, false);
@@ -109,7 +111,7 @@ class LibraryTemplateData extends TemplateData<Library> {
109111
@override
110112
String get title => '${library.name} library - Dart API';
111113
@override
112-
String get htmlBase => '..';
114+
String get htmlBase => '../';
113115
@override
114116
String get metaDescription =>
115117
'${library.name} library API docs, for the Dart programming language.';
@@ -164,7 +166,7 @@ class ClassTemplateData<T extends Class> extends TemplateData<T> {
164166
@override
165167
List get navLinks => [packageGraph.defaultPackage, library];
166168
@override
167-
String get htmlBase => '..';
169+
String get htmlBase => '../';
168170

169171
Class get objectType {
170172
if (_objectType != null) {
@@ -207,7 +209,7 @@ class ExtensionTemplateData<T extends Extension> extends TemplateData<T> {
207209
@override
208210
List get navLinks => [packageGraph.defaultPackage, library];
209211
@override
210-
String get htmlBase => '..';
212+
String get htmlBase => '../';
211213
}
212214

213215
class ConstructorTemplateData extends TemplateData<Constructor> {
@@ -235,7 +237,7 @@ class ConstructorTemplateData extends TemplateData<Constructor> {
235237
List get navLinksWithGenerics => [clazz];
236238
@override
237239
@override
238-
String get htmlBase => '../..';
240+
String get htmlBase => '../../';
239241
@override
240242
String get title => '${constructor.name} constructor - ${clazz.name} class - '
241243
'${library.name} library - Dart API';
@@ -279,7 +281,7 @@ class FunctionTemplateData extends TemplateData<ModelFunction> {
279281
@override
280282
List get navLinks => [packageGraph.defaultPackage, library];
281283
@override
282-
String get htmlBase => '..';
284+
String get htmlBase => '../';
283285
}
284286

285287
class MethodTemplateData extends TemplateData<Method> {
@@ -317,7 +319,7 @@ class MethodTemplateData extends TemplateData<Method> {
317319
@override
318320
List get navLinksWithGenerics => [container];
319321
@override
320-
String get htmlBase => '../..';
322+
String get htmlBase => '../../';
321323
}
322324

323325
class PropertyTemplateData extends TemplateData<Field> {
@@ -356,7 +358,7 @@ class PropertyTemplateData extends TemplateData<Field> {
356358
@override
357359
List get navLinksWithGenerics => [container];
358360
@override
359-
String get htmlBase => '../..';
361+
String get htmlBase => '../../';
360362

361363
String get type => 'property';
362364
}
@@ -400,7 +402,7 @@ class TypedefTemplateData extends TemplateData<Typedef> {
400402
@override
401403
List get navLinks => [packageGraph.defaultPackage, library];
402404
@override
403-
String get htmlBase => '..';
405+
String get htmlBase => '../';
404406
}
405407

406408
class TopLevelPropertyTemplateData extends TemplateData<TopLevelVariable> {
@@ -431,7 +433,7 @@ class TopLevelPropertyTemplateData extends TemplateData<TopLevelVariable> {
431433
@override
432434
List get navLinks => [packageGraph.defaultPackage, library];
433435
@override
434-
String get htmlBase => '..';
436+
String get htmlBase => '../';
435437

436438
String get _type => 'property';
437439
}

‎lib/src/model/package.dart‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,17 @@ import 'package:pub_semver/pub_semver.dart';
1414

1515
final RegExp substituteNameVersion = RegExp(r'%([bnv])%');
1616

17+
// All hrefs are emitted as relative paths from the output root. We are unable
18+
// to compute them from the page we are generating, and many properties computed
19+
// using hrefs are memoized anyway. To build complete relative hrefs, we emit
20+
// the href with this placeholder, and then replace it with the current page's
21+
// base href afterwards.
22+
// See https://github.com/dart-lang/dartdoc/issues/2090 for further context.
23+
// TODO: Find an approach that doesn't require doing this.
24+
// Unlikely to be mistaken for an identifier, html tag, or something else that
25+
// might reasonably exist normally.
26+
final String HTMLBASE_PLACEHOLDER = '\%\%__HTMLBASE_dartdoc_internal__\%\%';
27+
1728
/// A [LibraryContainer] that contains [Library] objects related to a particular
1829
/// package.
1930
class Package extends LibraryContainer
@@ -206,7 +217,7 @@ class Package extends LibraryContainer
206217
});
207218
if (!_baseHref.endsWith('/')) _baseHref = '${_baseHref}/';
208219
} else {
209-
_baseHref = '';
220+
_baseHref = config.useBaseHref ? '' : HTMLBASE_PLACEHOLDER;
210221
}
211222
}
212223
return _baseHref;

‎lib/templates/_footer.html‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@
1111
<!-- footer-text placeholder -->
1212
</footer>
1313

14+
{{! TODO(jdkoren): unwrap ^useBaseHref sections when the option is removed.}}
1415
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
15-
<script src="static-assets/typeahead.bundle.min.js"></script>
16-
<script src="static-assets/highlight.pack.js"></script>
17-
<script src="static-assets/URI.js"></script>
18-
<script src="static-assets/script.js"></script>
16+
<script src="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/typeahead.bundle.min.js"></script>
17+
<script src="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/highlight.pack.js"></script>
18+
<script src="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/URI.js"></script>
19+
<script src="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/script.js"></script>
1920
<!-- footer placeholder -->
2021

2122
</body>

‎lib/templates/_head.html‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,20 @@
1212
{{ #relCanonicalPrefix }}
1313
<link rel="canonical" href="{{{relCanonicalPrefix}}}/{{{self.href}}}">
1414
{{ /relCanonicalPrefix}}
15+
16+
{{#useBaseHref}}{{! TODO(jdkoren): remove when the useBaseHref option is removed.}}
1517
{{#htmlBase}}
1618
<!-- required because all the links are pseudo-absolute -->
1719
<base href="{{{htmlBase}}}">
1820
{{/htmlBase}}
21+
{{/useBaseHref}}
1922

23+
{{! TODO(jdkoren): unwrap ^useBaseHref sections when the option is removed.}}
2024
<link href="https://fonts.googleapis.com/css?family=Source+Code+Pro:500,400i,400,300|Source+Sans+Pro:400,300,700" rel="stylesheet">
2125
<link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">
22-
<link rel="stylesheet" href="static-assets/github.css">
23-
<link rel="stylesheet" href="static-assets/styles.css">
24-
<link rel="icon" href="static-assets/favicon.png">
26+
<link rel="stylesheet" href="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/github.css">
27+
<link rel="stylesheet" href="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/styles.css">
28+
<link rel="icon" href="{{^useBaseHref}}%%__HTMLBASE_dartdoc_internal__%%{{/useBaseHref}}static-assets/favicon.png">
2529
<!-- header placeholder -->
2630
</head>
2731

‎test/model_special_cases_test.dart‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,8 @@ void main() {
356356
hashCode.inheritance.any((c) => c.name == 'Interceptor'), isTrue);
357357
// If EventTarget really does start implementing hashCode, this will
358358
// fail.
359-
expect(hashCode.href, equals('dart-core/Object/hashCode.html'));
359+
expect(hashCode.href,
360+
equals('${HTMLBASE_PLACEHOLDER}dart-core/Object/hashCode.html'));
360361
expect(
361362
hashCode.canonicalEnclosingContainer, equals(objectModelElement));
362363
expect(
@@ -387,7 +388,8 @@ void main() {
387388
test('sdk library have formatted names', () {
388389
expect(dartAsyncLib.name, 'dart:async');
389390
expect(dartAsyncLib.dirName, 'dart-async');
390-
expect(dartAsyncLib.href, 'dart-async/dart-async-library.html');
391+
expect(dartAsyncLib.href,
392+
'${HTMLBASE_PLACEHOLDER}dart-async/dart-async-library.html');
391393
});
392394
});
393395

0 commit comments

Comments
 (0)