From d80c390dc31854405a99be3ec3b46d6af6c48d27 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Mon, 5 May 2025 22:28:25 -0500 Subject: [PATCH] [tool] Refactor WebTemplate to be immutable (#168201) This flows better. No uncertainty about calling the substitutions function more than once, etc. --- .../lib/src/build_system/targets/web.dart | 10 +- .../lib/src/isolated/devfs_web.dart | 30 +++--- .../flutter_tools/lib/src/web_template.dart | 37 ++++---- .../test/general.shard/web_template_test.dart | 95 ++++++++++--------- 4 files changed, 89 insertions(+), 83 deletions(-) diff --git a/packages/flutter_tools/lib/src/build_system/targets/web.dart b/packages/flutter_tools/lib/src/build_system/targets/web.dart index 7bc1012f6a0..e6775432fcf 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/web.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/web.dart @@ -571,7 +571,7 @@ _flutter.buildConfig = ${jsonEncode(buildConfig)}; // in question. final String? serviceWorkerVersion = includeServiceWorkerSettings ? Random().nextInt(1 << 32).toString() : null; - bootstrapTemplate.applySubstitutions( + final String bootstrapContent = bootstrapTemplate.withSubstitutions( baseHref: '', serviceWorkerVersion: serviceWorkerVersion, flutterJsFile: flutterJsFile, @@ -581,7 +581,7 @@ _flutter.buildConfig = ${jsonEncode(buildConfig)}; final File outputFlutterBootstrapJs = fileSystem.file( fileSystem.path.join(environment.outputDir.path, 'flutter_bootstrap.js'), ); - await outputFlutterBootstrapJs.writeAsString(bootstrapTemplate.content); + await outputFlutterBootstrapJs.writeAsString(bootstrapContent); await for (final FileSystemEntity file in webResources.list(recursive: true)) { if (file is File && file.basename == 'index.html') { @@ -592,18 +592,18 @@ _flutter.buildConfig = ${jsonEncode(buildConfig)}; _emitWebTemplateWarning(environment, relativePath, warning); } - indexHtmlTemplate.applySubstitutions( + final String indexHtmlContent = indexHtmlTemplate.withSubstitutions( baseHref: environment.defines[kBaseHref] ?? '/', serviceWorkerVersion: serviceWorkerVersion, flutterJsFile: flutterJsFile, buildConfig: buildConfig, - flutterBootstrapJs: bootstrapTemplate.content, + flutterBootstrapJs: bootstrapContent, ); final File outputIndexHtml = fileSystem.file( fileSystem.path.join(environment.outputDir.path, relativePath), ); await outputIndexHtml.create(recursive: true); - await outputIndexHtml.writeAsString(indexHtmlTemplate.content); + await outputIndexHtml.writeAsString(indexHtmlContent); } } } diff --git a/packages/flutter_tools/lib/src/isolated/devfs_web.dart b/packages/flutter_tools/lib/src/isolated/devfs_web.dart index a70cf04e189..e425d092f09 100644 --- a/packages/flutter_tools/lib/src/isolated/devfs_web.dart +++ b/packages/flutter_tools/lib/src/isolated/devfs_web.dart @@ -132,7 +132,7 @@ class WebAssetServer implements AssetReader { this._canaryFeatures, { required this.webRenderer, required this.useLocalCanvasKit, - }) : basePath = _getWebTemplate('index.html', _kDefaultIndex).getBaseHref() { + }) : basePath = WebTemplate.baseHref(_htmlTemplate('index.html', _kDefaultIndex)) { // TODO(srujzs): Remove this assertion when the library bundle format is // supported without canary mode. if (_ddcModuleSystem) { @@ -671,13 +671,12 @@ _flutter.buildConfig = ${jsonEncode(buildConfig)}; 'flutter_bootstrap.js', generateDefaultFlutterBootstrapScript(includeServiceWorkerSettings: false), ); - bootstrapTemplate.applySubstitutions( + return bootstrapTemplate.withSubstitutions( baseHref: '/', serviceWorkerVersion: null, buildConfig: _buildConfigString, flutterJsFile: _flutterJsFile, ); - return bootstrapTemplate.content; } shelf.Response _serveFlutterBootstrapJs() { @@ -689,16 +688,15 @@ _flutter.buildConfig = ${jsonEncode(buildConfig)}; shelf.Response _serveIndexHtml() { final WebTemplate indexHtml = _getWebTemplate('index.html', _kDefaultIndex); - indexHtml.applySubstitutions( - // Currently, we don't support --base-href for the "run" command. - baseHref: '/', - serviceWorkerVersion: null, - buildConfig: _buildConfigString, - flutterJsFile: _flutterJsFile, - flutterBootstrapJs: _flutterBootstrapJsContent, - ); return shelf.Response.ok( - indexHtml.content, + indexHtml.withSubstitutions( + // Currently, we don't support --base-href for the "run" command. + baseHref: '/', + serviceWorkerVersion: null, + buildConfig: _buildConfigString, + flutterJsFile: _flutterJsFile, + flutterBootstrapJs: _flutterBootstrapJsContent, + ), headers: {HttpHeaders.contentTypeHeader: 'text/html'}, ); } @@ -1375,7 +1373,11 @@ String? _stripBasePath(String path, String basePath) { } WebTemplate _getWebTemplate(String filename, String fallbackContent) { - final File template = globals.fs.currentDirectory.childDirectory('web').childFile(filename); - final String htmlContent = template.existsSync() ? template.readAsStringSync() : fallbackContent; + final String htmlContent = _htmlTemplate(filename, fallbackContent); return WebTemplate(htmlContent); } + +String _htmlTemplate(String filename, String fallbackContent) { + final File template = globals.fs.currentDirectory.childDirectory('web').childFile(filename); + return template.existsSync() ? template.readAsStringSync() : fallbackContent; +} diff --git a/packages/flutter_tools/lib/src/web_template.dart b/packages/flutter_tools/lib/src/web_template.dart index 2ebf6db76d9..7756d6a4e9f 100644 --- a/packages/flutter_tools/lib/src/web_template.dart +++ b/packages/flutter_tools/lib/src/web_template.dart @@ -4,6 +4,7 @@ import 'package:html/dom.dart'; import 'package:html/parser.dart'; +import 'package:meta/meta.dart'; import 'base/common.dart'; import 'base/file_system.dart'; @@ -29,16 +30,12 @@ class WebTemplateWarning { /// } /// ``` class WebTemplate { - WebTemplate(this._content); + const WebTemplate(this._content); - String get content => _content; - String _content; + final String _content; - Document _getDocument() => parse(_content); - - /// Parses the base href from the index.html file. - String getBaseHref() { - final Element? baseElement = _getDocument().querySelector('base'); + static String baseHref(String html) { + final Element? baseElement = parse(html).querySelector('base'); final String? baseHref = baseElement?.attributes == null ? null : baseElement!.attributes['href']; @@ -94,20 +91,23 @@ class WebTemplate { return WebTemplateWarning(warningText, lineCount + 1); } - /// Applies substitutions to the content of the index.html file. - void applySubstitutions({ + /// Applies substitutions to the content of the index.html file and returns the result. + @useResult + String withSubstitutions({ required String baseHref, required String? serviceWorkerVersion, required File flutterJsFile, String? buildConfig, String? flutterBootstrapJs, }) { - if (_content.contains(kBaseHrefPlaceholder)) { - _content = _content.replaceAll(kBaseHrefPlaceholder, baseHref); + String newContent = _content; + + if (newContent.contains(kBaseHrefPlaceholder)) { + newContent = newContent.replaceAll(kBaseHrefPlaceholder, baseHref); } if (serviceWorkerVersion != null) { - _content = _content + newContent = newContent .replaceFirst( // Support older `var` syntax as well as new `const` syntax RegExp('(const|var) serviceWorkerVersion = null'), @@ -120,21 +120,22 @@ class WebTemplate { "navigator.serviceWorker.register('flutter_service_worker.js?v=$serviceWorkerVersion')", ); } - _content = _content.replaceAll( + newContent = newContent.replaceAll( '{{flutter_service_worker_version}}', serviceWorkerVersion != null ? '"$serviceWorkerVersion"' : 'null', ); if (buildConfig != null) { - _content = _content.replaceAll('{{flutter_build_config}}', buildConfig); + newContent = newContent.replaceAll('{{flutter_build_config}}', buildConfig); } - if (_content.contains('{{flutter_js}}')) { - _content = _content.replaceAll('{{flutter_js}}', flutterJsFile.readAsStringSync()); + if (newContent.contains('{{flutter_js}}')) { + newContent = newContent.replaceAll('{{flutter_js}}', flutterJsFile.readAsStringSync()); } if (flutterBootstrapJs != null) { - _content = _content.replaceAll('{{flutter_bootstrap_js}}', flutterBootstrapJs); + newContent = newContent.replaceAll('{{flutter_bootstrap_js}}', flutterBootstrapJs); } + return newContent; } } diff --git a/packages/flutter_tools/test/general.shard/web_template_test.dart b/packages/flutter_tools/test/general.shard/web_template_test.dart index b30ad4380f5..8e9d81a6064 100644 --- a/packages/flutter_tools/test/general.shard/web_template_test.dart +++ b/packages/flutter_tools/test/general.shard/web_template_test.dart @@ -221,93 +221,96 @@ void main() { flutterJs.writeAsStringSync('(flutter.js content)'); test('can parse baseHref', () { - expect(WebTemplate('').getBaseHref(), 'foo/111'); - expect(WebTemplate(htmlSample1).getBaseHref(), 'foo/222'); - expect(WebTemplate(htmlSample2).getBaseHref(), ''); // Placeholder base href. + expect(WebTemplate.baseHref(''), 'foo/111'); + expect(WebTemplate.baseHref(htmlSample1), 'foo/222'); + expect(WebTemplate.baseHref(htmlSample2), ''); // Placeholder base href. }); test('handles missing baseHref', () { - expect(WebTemplate('').getBaseHref(), ''); - expect(WebTemplate('').getBaseHref(), ''); - expect(WebTemplate(htmlSample3).getBaseHref(), ''); + expect(WebTemplate.baseHref(''), ''); + expect(WebTemplate.baseHref(''), ''); + expect(WebTemplate.baseHref(htmlSample3), ''); }); test('throws on invalid baseHref', () { - expect(() => WebTemplate('').getBaseHref(), throwsToolExit()); - expect(() => WebTemplate('').getBaseHref(), throwsToolExit()); - expect(() => WebTemplate('').getBaseHref(), throwsToolExit()); - expect(() => WebTemplate('').getBaseHref(), throwsToolExit()); - expect(() => WebTemplate('').getBaseHref(), throwsToolExit()); + expect(() => WebTemplate.baseHref(''), throwsToolExit()); + expect(() => WebTemplate.baseHref(''), throwsToolExit()); + expect(() => WebTemplate.baseHref(''), throwsToolExit()); + expect(() => WebTemplate.baseHref(''), throwsToolExit()); + expect(() => WebTemplate.baseHref(''), throwsToolExit()); }); test('applies substitutions', () { - final WebTemplate indexHtml = WebTemplate(htmlSample2); - indexHtml.applySubstitutions( - baseHref: '/foo/333/', - serviceWorkerVersion: 'v123xyz', - flutterJsFile: flutterJs, - ); + const WebTemplate indexHtml = WebTemplate(htmlSample2); + expect( - indexHtml.content, + indexHtml.withSubstitutions( + baseHref: '/foo/333/', + serviceWorkerVersion: 'v123xyz', + flutterJsFile: flutterJs, + ), htmlSample2Replaced(baseHref: '/foo/333/', serviceWorkerVersion: 'v123xyz'), ); }); test('applies substitutions with legacy var version syntax', () { - final WebTemplate indexHtml = WebTemplate(htmlSampleLegacyVar); - indexHtml.applySubstitutions( - baseHref: '/foo/333/', - serviceWorkerVersion: 'v123xyz', - flutterJsFile: flutterJs, - ); + const WebTemplate indexHtml = WebTemplate(htmlSampleLegacyVar); expect( - indexHtml.content, + indexHtml.withSubstitutions( + baseHref: '/foo/333/', + serviceWorkerVersion: 'v123xyz', + flutterJsFile: flutterJs, + ), htmlSample2Replaced(baseHref: '/foo/333/', serviceWorkerVersion: 'v123xyz'), ); }); test('applies substitutions to inline flutter.js bootstrap script', () { - final WebTemplate indexHtml = WebTemplate(htmlSampleInlineFlutterJsBootstrap); + const WebTemplate indexHtml = WebTemplate(htmlSampleInlineFlutterJsBootstrap); expect(indexHtml.getWarnings(), isEmpty); - indexHtml.applySubstitutions( - baseHref: '/', - serviceWorkerVersion: '(service worker version)', - flutterJsFile: flutterJs, - buildConfig: '(build config)', + expect( + indexHtml.withSubstitutions( + baseHref: '/', + serviceWorkerVersion: '(service worker version)', + flutterJsFile: flutterJs, + buildConfig: '(build config)', + ), + htmlSampleInlineFlutterJsBootstrapOutput, ); - expect(indexHtml.content, htmlSampleInlineFlutterJsBootstrapOutput); }); test('applies substitutions to full flutter_bootstrap.js replacement', () { - final WebTemplate indexHtml = WebTemplate(htmlSampleFullFlutterBootstrapReplacement); + const WebTemplate indexHtml = WebTemplate(htmlSampleFullFlutterBootstrapReplacement); expect(indexHtml.getWarnings(), isEmpty); - indexHtml.applySubstitutions( - baseHref: '/', - serviceWorkerVersion: '(service worker version)', - flutterJsFile: flutterJs, - buildConfig: '(build config)', - flutterBootstrapJs: '(flutter bootstrap script)', + expect( + indexHtml.withSubstitutions( + baseHref: '/', + serviceWorkerVersion: '(service worker version)', + flutterJsFile: flutterJs, + buildConfig: '(build config)', + flutterBootstrapJs: '(flutter bootstrap script)', + ), + htmlSampleFullFlutterBootstrapReplacementOutput, ); - expect(indexHtml.content, htmlSampleFullFlutterBootstrapReplacementOutput); }); test('re-parses after substitutions', () { - final WebTemplate indexHtml = WebTemplate(htmlSample2); - expect(indexHtml.getBaseHref(), ''); // Placeholder base href. + const WebTemplate indexHtml = WebTemplate(htmlSample2); + expect(WebTemplate.baseHref(htmlSample2), ''); // Placeholder base href. - indexHtml.applySubstitutions( + final String substituted = indexHtml.withSubstitutions( baseHref: '/foo/333/', serviceWorkerVersion: 'v123xyz', flutterJsFile: flutterJs, ); // The parsed base href should be updated after substitutions. - expect(indexHtml.getBaseHref(), 'foo/333'); + expect(WebTemplate.baseHref(substituted), 'foo/333'); }); test('warns on legacy service worker patterns', () { - final WebTemplate indexHtml = WebTemplate(htmlSampleLegacyVar); + const WebTemplate indexHtml = WebTemplate(htmlSampleLegacyVar); final List warnings = indexHtml.getWarnings(); expect(warnings.length, 2); @@ -316,7 +319,7 @@ void main() { }); test('warns on legacy FlutterLoader.loadEntrypoint', () { - final WebTemplate indexHtml = WebTemplate(htmlSampleLegacyLoadEntrypoint); + const WebTemplate indexHtml = WebTemplate(htmlSampleLegacyLoadEntrypoint); final List warnings = indexHtml.getWarnings(); expect(warnings.length, 1);