From f8a1b34d600fd0a7fd72de482c563fb8c4ea10d3 Mon Sep 17 00:00:00 2001 From: EricApostal <60072374+EricApostal@users.noreply.github.com> Date: Thu, 12 Feb 2026 22:16:37 -0500 Subject: [PATCH] Don't throw an exception if no web define variable is set (#182273) This PR fixes a problem introduced in https://github.com/flutter/flutter/pull/175805 that caused a build to exception to be thrown if variables like `{{this}}` were defined in the web `index.html`, but were not explicitly set by `--web-define=this=VALUE`. Resolves https://github.com/flutter/flutter/issues/182243 Note: As per https://github.com/flutter/flutter/issues/182076, this will always show the warning if ran with `flutter build web`, whereas `flutter run -d ...` works fine, regardless of whether you've set the variable or not. That is set to be fixed in https://github.com/flutter/flutter/pull/182079, and does not have to do with this PR. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Navaron Bracke --- .../lib/src/build_system/targets/web.dart | 2 + .../lib/src/isolated/web_asset_server.dart | 5 ++ .../lib/src/runner/flutter_command.dart | 2 +- .../flutter_tools/lib/src/web_template.dart | 24 ++++-- .../web/devfs_web_ddc_modules_test.dart | 6 ++ .../general.shard/web/devfs_web_test.dart | 84 ++++++++++++++++++- .../test/general.shard/web_template_test.dart | 74 ++++++++++------ 7 files changed, 158 insertions(+), 39 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 b90d21a5200..3f0bed672cd 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/web.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/web.dart @@ -745,6 +745,7 @@ _flutter.buildConfig = ${jsonEncode(buildConfig)}; serviceWorkerVersion: serviceWorkerVersion, flutterJsFile: flutterJsFile, buildConfig: buildConfig, + logger: environment.logger, webDefines: webDefines, ); @@ -769,6 +770,7 @@ _flutter.buildConfig = ${jsonEncode(buildConfig)}; flutterJsFile: flutterJsFile, buildConfig: buildConfig, flutterBootstrapJs: bootstrapContent, + logger: environment.logger, webDefines: webDefines, ); final File outputIndexHtml = fileSystem.file( diff --git a/packages/flutter_tools/lib/src/isolated/web_asset_server.dart b/packages/flutter_tools/lib/src/isolated/web_asset_server.dart index 6f987c1ba41..639501bc23c 100644 --- a/packages/flutter_tools/lib/src/isolated/web_asset_server.dart +++ b/packages/flutter_tools/lib/src/isolated/web_asset_server.dart @@ -79,6 +79,7 @@ class WebAssetServer implements AssetReader { required this.webRenderer, required this.useLocalCanvasKit, required this.fileSystem, + required this.logger, Map webDefines = const {}, }) : basePath = WebTemplate.baseHref(htmlTemplate(fileSystem, 'index.html', _kDefaultIndex)), _webDefines = webDefines { @@ -266,6 +267,7 @@ class WebAssetServer implements AssetReader { webRenderer: webRenderer, useLocalCanvasKit: useLocalCanvasKit, fileSystem: fileSystem, + logger: logger, webDefines: webDefines, ); final int selectedPort = server.selectedPort; @@ -595,6 +597,7 @@ class WebAssetServer implements AssetReader { final bool useLocalCanvasKit; final FileSystem fileSystem; + final Logger logger; String get _buildConfigString { final buildConfig = { @@ -634,6 +637,7 @@ _flutter.buildConfig = ${jsonEncode(buildConfig)}; serviceWorkerVersion: null, buildConfig: _buildConfigString, flutterJsFile: _flutterJsFile, + logger: logger, webDefines: _webDefines, ); } @@ -657,6 +661,7 @@ _flutter.buildConfig = ${jsonEncode(buildConfig)}; buildConfig: _buildConfigString, flutterJsFile: _flutterJsFile, flutterBootstrapJs: _flutterBootstrapJsContent, + logger: logger, webDefines: _webDefines, ), encoding: utf8, diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 091be82f457..74c50ead025 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -774,7 +774,7 @@ abstract class FlutterCommand extends Command { 'Variables are replaced in the format {{VARIABLE_NAME}}.\n' 'Multiple defines can be passed by repeating "--${FlutterOptions.kWebDefinesOption}" multiple times.\n' 'If a template contains a variable placeholder but no corresponding "--web-define" is provided, ' - 'the build will fail with an error.', + 'it will warn that you have an unhandled variable.', valueHelp: 'API_URL=https://api.example.com', splitCommas: false, ); diff --git a/packages/flutter_tools/lib/src/web_template.dart b/packages/flutter_tools/lib/src/web_template.dart index 919f3eefc19..2910236167a 100644 --- a/packages/flutter_tools/lib/src/web_template.dart +++ b/packages/flutter_tools/lib/src/web_template.dart @@ -8,6 +8,7 @@ import 'package:meta/meta.dart'; import 'base/common.dart'; import 'base/file_system.dart'; +import 'base/logger.dart'; import 'base/utils.dart'; /// Placeholder for base href @@ -107,6 +108,7 @@ class WebTemplate { String? buildConfig, String? flutterBootstrapJs, String? staticAssetsUrl, + required Logger logger, Map webDefines = const {}, }) { String newContent = _content; @@ -133,7 +135,7 @@ class WebTemplate { "navigator.serviceWorker.register('flutter_service_worker.js?v=$serviceWorkerVersion') /* $_kServiceWorkerDeprecationNotice */", ); } - newContent = _applyVariableSubstitutions(newContent, { + newContent = _applyVariableSubstitutions(newContent, logger, { ...webDefines, if (buildConfig != null) 'flutter_build_config': buildConfig, if (flutterBootstrapJs != null) 'flutter_bootstrap_js': flutterBootstrapJs, @@ -149,8 +151,13 @@ class WebTemplate { /// Applies web-define variable substitutions and validates all variables are provided. /// /// Replaces {{VARIABLE}} placeholders with values from webDefines. Built-in Flutter - /// variables are preserved if missing; user-defined variables throw ToolExit. - String _applyVariableSubstitutions(String content, Map webDefines) { + /// variables are preserved if missing; user-defined variables will log a warning + /// and be skipped. + String _applyVariableSubstitutions( + String content, + Logger logger, + Map webDefines, + ) { final variablePattern = RegExp(r'\{\{([A-Za-z_][A-Za-z0-9_]*)\}\}'); final missingVariables = {}; @@ -188,13 +195,16 @@ class WebTemplate { .map((String name) => '--web-define=$name=VALUE') .join(' '); final String variablesList = pluralize('variable', missingVariables.length); - throwToolExit( - 'Missing web-define $variablesList: $variables\n\n' - 'Please provide the missing $variablesList using:\n' + logger.printWarning( + 'Warning: Missing web-define $variablesList: $variables\n\n' + 'You can provide the missing $variablesList using:\n' 'flutter run $suggestion\n' 'or\n' - 'flutter build web $suggestion', + 'flutter build web $suggestion\n' + 'This variable will be skipped.\n', ); + + return result; } } diff --git a/packages/flutter_tools/test/general.shard/web/devfs_web_ddc_modules_test.dart b/packages/flutter_tools/test/general.shard/web/devfs_web_ddc_modules_test.dart index 3a0a1184d21..682792b4f2a 100644 --- a/packages/flutter_tools/test/general.shard/web/devfs_web_ddc_modules_test.dart +++ b/packages/flutter_tools/test/general.shard/web/devfs_web_ddc_modules_test.dart @@ -81,6 +81,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ); releaseAssetServer = ReleaseAssetServer( globals.fs.file('main.dart').uri, @@ -329,6 +330,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ); expect(webAssetServer.basePath, 'foo/bar'); @@ -350,6 +352,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ); // Defaults to "/" when there's no base element. @@ -373,6 +376,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ), throwsToolExit(), ); @@ -395,6 +399,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ), throwsToolExit(), ); @@ -1216,6 +1221,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ); expect(await webAssetServer.metadataContents('foo/main_module.ddc_merged_metadata'), null); diff --git a/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart b/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart index 61996302213..195e5c29ef4 100644 --- a/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart +++ b/packages/flutter_tools/test/general.shard/web/devfs_web_test.dart @@ -80,6 +80,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ); releaseAssetServer = ReleaseAssetServer( globals.fs.file('main.dart').uri, @@ -352,6 +353,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ); expect(webAssetServer.basePath, 'foo/bar'); @@ -376,6 +378,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ); // Defaults to "/" when there's no base element. @@ -402,6 +405,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ), throwsToolExit(), ); @@ -427,6 +431,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ), throwsToolExit(), ); @@ -500,6 +505,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: true, fileSystem: globals.fs, + logger: logger, ); final Response response = await webAssetServer.handleRequest( @@ -1530,6 +1536,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ); expect(await webAssetServer.metadataContents('foo/main_module.ddc_merged_metadata'), null); @@ -1596,7 +1603,7 @@ void main() { ); test( - 'WebAssetServer serves index.html with web-define variables', + 'WebAssetServer serves index.html without user defined web-define variables', () => testbed.run(() async { // Simple test case with no custom variables - should work like before globals.fs.file( @@ -1619,6 +1626,7 @@ void main() { webRenderer: WebRendererMode.canvaskit, useLocalCanvasKit: false, fileSystem: globals.fs, + logger: logger, ); final Response response = await webAssetServer.handleRequest( @@ -1629,7 +1637,7 @@ void main() { ); test( - 'WebAssetServer throws error for missing web-define variables in index.html', + 'WebAssetServer warns for missing user defined web-define variables in index.html', () => testbed.run(() async { const htmlContent = ''' @@ -1670,11 +1678,78 @@ void main() { useLocalCanvasKit: false, fileSystem: globals.fs, webDefines: {}, // Empty webDefines + logger: logger, ); + final Response response = await webAssetServer.handleRequest( + Request('GET', Uri.parse('http://foobar/')), + ); + + expect(response.statusCode, HttpStatus.ok); + // Verify the placeholder is preserved + expect(await response.readAsString(), contains("const apiUrl = '{{MISSING_VAR}}';")); + expect(logger.warningText, contains('Missing web-define variable: MISSING_VAR')); + }), + ); + + test( + 'WebAssetServer logs warning for multiple missing web-define variables in index.html', + () => testbed.run(() async { + const htmlContent = ''' + + + + Test + + + + + +'''; + + globals.fs.currentDirectory.childDirectory('web').childFile('index.html') + ..createSync(recursive: true) + ..writeAsStringSync(htmlContent); + + globals.fs.file( + globals.fs.path.join( + globals.artifacts!.getHostArtifact(HostArtifact.flutterJsDirectory).path, + 'flutter.js', + ), + ) + ..createSync(recursive: true) + ..writeAsStringSync('flutter.js content'); + + final webAssetServer = WebAssetServer( + FakeHttpServer(), + PackageConfig.empty, + InternetAddress.anyIPv4, + {}, + {}, + usesDdcModuleSystem, + canaryFeatures, + webRenderer: WebRendererMode.canvaskit, + useLocalCanvasKit: false, + fileSystem: globals.fs, + webDefines: {}, // Empty webDefines + logger: logger, + ); + + final Response response = await webAssetServer.handleRequest( + Request('GET', Uri.parse('http://foobar/')), + ); + + expect(response.statusCode, HttpStatus.ok); + // Verify the placeholders are preserved + final String responseBody = await response.readAsString(); + expect(responseBody, contains("const apiUrl = '{{MISSING_VAR_1}}';")); + expect(responseBody, contains("const apiKey = '{{MISSING_VAR_2}}';")); expect( - () async => webAssetServer.handleRequest(Request('GET', Uri.parse('http://foobar/'))), - throwsToolExit(message: 'Missing web-define variable: MISSING_VAR'), + logger.warningText, + contains('Missing web-define variables: MISSING_VAR_1, MISSING_VAR_2'), ); }), ); @@ -1715,6 +1790,7 @@ const config = { useLocalCanvasKit: false, fileSystem: globals.fs, webDefines: {'API_URL': 'https://test.api.com', 'DEBUG_MODE': 'true'}, + logger: logger, ); final Response response = await webAssetServer.handleRequest( 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 a2b03907321..63b61a34561 100644 --- a/packages/flutter_tools/test/general.shard/web_template_test.dart +++ b/packages/flutter_tools/test/general.shard/web_template_test.dart @@ -4,9 +4,12 @@ import 'package:file/file.dart'; import 'package:file/memory.dart'; + +import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/web_template.dart'; import '../src/common.dart'; +import '../src/context.dart'; const htmlSample1 = ''' @@ -285,6 +288,7 @@ String htmlSampleStaticAssetsUrlReplaced({required String staticAssetsUrl}) => void main() { final fs = MemoryFileSystem(); + final logger = BufferLogger.test(); final File flutterJs = fs.file('flutter.js'); flutterJs.writeAsStringSync('(flutter.js content)'); @@ -316,6 +320,7 @@ void main() { baseHref: '/foo/333/', serviceWorkerVersion: 'v123xyz', flutterJsFile: flutterJs, + logger: logger, ), htmlSample2Replaced(baseHref: '/foo/333/', serviceWorkerVersion: 'v123xyz'), ); @@ -328,6 +333,7 @@ void main() { baseHref: '/foo/333/', serviceWorkerVersion: 'v123xyz', flutterJsFile: flutterJs, + logger: logger, ), htmlSample2Replaced(baseHref: '/foo/333/', serviceWorkerVersion: 'v123xyz'), ); @@ -343,6 +349,7 @@ void main() { serviceWorkerVersion: '(service worker version)', flutterJsFile: flutterJs, buildConfig: '(build config)', + logger: logger, ), htmlSampleInlineFlutterJsBootstrapOutput, ); @@ -359,6 +366,7 @@ void main() { flutterJsFile: flutterJs, buildConfig: '(build config)', flutterBootstrapJs: '(flutter bootstrap script)', + logger: logger, ), htmlSampleFullFlutterBootstrapReplacementOutput, ); @@ -374,6 +382,7 @@ void main() { serviceWorkerVersion: 'v123xyz', flutterJsFile: flutterJs, staticAssetsUrl: expectedStaticAssetsUrl, + logger: logger, ), htmlSampleStaticAssetsUrlReplaced(staticAssetsUrl: expectedStaticAssetsUrl), ); @@ -387,6 +396,7 @@ void main() { baseHref: '/foo/333/', serviceWorkerVersion: 'v123xyz', flutterJsFile: flutterJs, + logger: logger, ); // The parsed base href should be updated after substitutions. expect(WebTemplate.baseHref(substituted), 'foo/333'); @@ -452,6 +462,7 @@ void main() { 'ENV': 'production', 'DEBUG_MODE': 'false', }, + logger: logger, ); expect(result, contains("apiUrl: 'https://api.example.com'")); @@ -459,7 +470,7 @@ void main() { expect(result, contains('debugMode: false')); }); - test('throws ToolExit when web-define variable is missing', () { + testUsingContext('logs warning when user defined web-define variable is missing', () { const htmlWithMissingVar = ''' @@ -475,18 +486,20 @@ void main() { '''; const indexHtml = WebTemplate(htmlWithMissingVar); - expect( - () => indexHtml.withSubstitutions( - baseHref: '/', - serviceWorkerVersion: null, - flutterJsFile: flutterJs, - webDefines: {}, // Missing API_URL - ), - throwsToolExit(message: 'Missing web-define variable: API_URL'), + final String result = indexHtml.withSubstitutions( + baseHref: '/', + serviceWorkerVersion: null, + flutterJsFile: flutterJs, + webDefines: {}, // Missing API_URL + logger: testLogger, ); + + expect(testLogger.warningText, contains('Missing web-define variable: API_URL')); + // Verify the placeholder is preserved + expect(result, contains("const apiUrl = '{{API_URL}}';")); }); - test('throws ToolExit with multiple missing variables', () { + testUsingContext('logs warning with multiple missing user defined variables', () { const htmlWithMultipleMissingVars = ''' @@ -506,19 +519,23 @@ void main() { '''; const indexHtml = WebTemplate(htmlWithMultipleMissingVars); - expect( - () => indexHtml.withSubstitutions( - baseHref: '/', - serviceWorkerVersion: null, - flutterJsFile: flutterJs, - webDefines: {'API_URL': 'test'}, // Missing ENV, VERSION - ), - throwsToolExit(message: 'Missing web-define variable'), + final String result = indexHtml.withSubstitutions( + baseHref: '/', + serviceWorkerVersion: null, + flutterJsFile: flutterJs, + webDefines: {'API_URL': 'test'}, // Missing ENV, VERSION + logger: testLogger, ); + + expect(testLogger.warningText, contains('Missing web-define variables: ENV, VERSION')); + expect(result, contains("env: '{{ENV}}'")); + expect(result, contains("version: '{{VERSION}}'")); }); - test('ignores Flutter built-in variables when validating web-define variables', () { - const htmlWithBuiltInVars = ''' + testUsingContext( + 'ignores Flutter built-in variables and logs warning for missing user variables', + () { + const htmlWithBuiltInVars = ''' @@ -534,18 +551,20 @@ void main() { '''; - const indexHtml = WebTemplate(htmlWithBuiltInVars); - expect( - () => indexHtml.withSubstitutions( + const indexHtml = WebTemplate(htmlWithBuiltInVars); + final String result = indexHtml.withSubstitutions( baseHref: '/', serviceWorkerVersion: null, flutterJsFile: flutterJs, buildConfig: 'test config', webDefines: {}, // Missing CUSTOM_VAR but built-in vars should be ignored - ), - throwsToolExit(message: 'Missing web-define variable: CUSTOM_VAR'), - ); - }); + logger: testLogger, + ); + + expect(testLogger.warningText, contains('Missing web-define variable: CUSTOM_VAR')); + expect(result, contains("const customVar = '{{CUSTOM_VAR}}';")); + }, + ); test('allows empty web-define variables', () { const htmlWithEmptyVar = ''' @@ -568,6 +587,7 @@ void main() { serviceWorkerVersion: null, flutterJsFile: flutterJs, webDefines: {'EMPTY_VAR': ''}, + logger: logger, ); expect(result, contains("const value = '';"));