From e5d200fda5006060dac451aae443108f08aedd20 Mon Sep 17 00:00:00 2001 From: Guillaume Bernos Date: Wed, 6 May 2026 15:23:21 +0200 Subject: [PATCH 1/3] fix: discover only initialized Dart functions --- lib/builder.dart | 79 +++++++++++++++++++- test/fixtures/dart_reference/bin/server.dart | 10 +++ test/snapshots/manifest_snapshot_test.dart | 7 ++ 3 files changed, 92 insertions(+), 4 deletions(-) diff --git a/lib/builder.dart b/lib/builder.dart index 539fb88..80325e0 100644 --- a/lib/builder.dart +++ b/lib/builder.dart @@ -269,26 +269,44 @@ class _FirebaseFunctionsVisitor extends RecursiveAstVisitor { /// firebase.https.onRequest(name: 'fn', options: opts, ...); final Map _variableToOptionsExpr = {}; + /// Top-level function declarations, used to follow helpers that are invoked + /// from the `runFunctions`/`fireUp` registration callback. + final Map _topLevelFunctions = {}; + + final Set _visitedRegistrationHelpers = {}; + + var _registrationDepth = 0; + + bool get _isCollectingRegistrations => _registrationDepth > 0; + @override void visitMethodInvocation(MethodInvocation node) { - super.visitMethodInvocation(node); final target = node.target; final methodName = node.methodName.name; - if (target != null) { + + if (_isFunctionsRunnerInvocation(methodName)) { + _visitRegistrationRunner(node); + return; + } + + if (_isCollectingRegistrations && target != null) { // Check against all namespaces for (final namespace in namespaces) { if (namespace.isNamespace(target)) { if (namespace.matches(methodName)) { namespace.extractor(node, methodName); - // Found a match, no need to check other namespaces for this node + // The function handler runs later, so don't scan its body as if it + // were part of initialization. return; } } } - } else { + } else if (target == null) { // Check for parameter definitions (top-level function calls with no target) if (_isParamDefinition(methodName)) { _extractParameterFromMethod(node, methodName); + } else if (_isCollectingRegistrations) { + _visitRegistrationHelper(methodName); } } @@ -300,6 +318,11 @@ class _FirebaseFunctionsVisitor extends RecursiveAstVisitor { if (node.function case final SimpleIdentifier function) { final functionName = function.name; + if (_isFunctionsRunnerInvocation(functionName)) { + _visitRegistrationRunnerArguments(functionName, node.argumentList); + return; + } + // Check for parameter definitions if (_isParamDefinition(functionName)) { _extractParameter(node, functionName); @@ -317,11 +340,59 @@ class _FirebaseFunctionsVisitor extends RecursiveAstVisitor { for (final declaration in node.declarations) { if (declaration is TopLevelVariableDeclaration) { _trackOptionsVariables(declaration.variables); + } else if (declaration is FunctionDeclaration) { + _topLevelFunctions[declaration.name.lexeme] = declaration; } } super.visitCompilationUnit(node); } + bool _isFunctionsRunnerInvocation(String methodName) => + methodName == 'runFunctions' || methodName == 'fireUp'; + + void _visitRegistrationRunner(MethodInvocation node) { + _visitRegistrationRunnerArguments(node.methodName.name, node.argumentList); + } + + void _visitRegistrationRunnerArguments(String runnerName, ArgumentList args) { + final arguments = args.arguments; + final runnerArgIndex = runnerName == 'fireUp' ? 1 : 0; + if (arguments.length <= runnerArgIndex) return; + + _visitRegistrationExpression(arguments[runnerArgIndex]); + } + + void _visitRegistrationExpression(Expression expression) { + switch (expression) { + case FunctionExpression(:final body): + _visitCollectingRegistrations(() => body.accept(this)); + case SimpleIdentifier(:final name): + _visitRegistrationHelper(name); + case ParenthesizedExpression(:final expression): + _visitRegistrationExpression(expression); + } + } + + void _visitRegistrationHelper(String name) { + if (!_visitedRegistrationHelpers.add(name)) return; + + final declaration = _topLevelFunctions[name]; + if (declaration == null) return; + + _visitCollectingRegistrations(() { + declaration.functionExpression.body.accept(this); + }); + } + + void _visitCollectingRegistrations(void Function() visit) { + _registrationDepth++; + try { + visit(); + } finally { + _registrationDepth--; + } + } + @override void visitVariableDeclarationStatement(VariableDeclarationStatement node) { // Track local variable declarations (e.g., `const opts = HttpsOptions(...)` diff --git a/test/fixtures/dart_reference/bin/server.dart b/test/fixtures/dart_reference/bin/server.dart index 5867232..de5ec19 100644 --- a/test/fixtures/dart_reference/bin/server.dart +++ b/test/fixtures/dart_reference/bin/server.dart @@ -768,6 +768,16 @@ void main(List args) async { }); } +/// This helper is intentionally not called from [main]. The manifest builder +/// should only discover registrations reached during `runFunctions` setup. +// ignore: unreachable_from_main +void unregisteredHelper(Firebase firebase) { + firebase.https.onRequest( + name: 'unregisteredHelper', + (request) async => Response.ok('This function should not be discovered'), + ); +} + /// Options assigned to a top-level const variable. const httpsVarOpts = HttpsOptions( region: Region(SupportedRegion.europeWest3), diff --git a/test/snapshots/manifest_snapshot_test.dart b/test/snapshots/manifest_snapshot_test.dart index 1e24b93..6500ae9 100644 --- a/test/snapshots/manifest_snapshot_test.dart +++ b/test/snapshots/manifest_snapshot_test.dart @@ -351,6 +351,13 @@ void main() { } }); + test('should ignore uncalled top-level registration helpers', () { + final dartEndpoints = dartManifest['endpoints'] as Map; + + expect(dartEndpoints, isNot(contains('unregisteredHelper'))); + expect(dartEndpoints, isNot(contains('unregistered-helper'))); + }); + // ========================================================================= // Callable Functions Tests // ========================================================================= From 3bfb4c725267bbbfdeb5401288fb3b158029e67c Mon Sep 17 00:00:00 2001 From: Guillaume Bernos Date: Wed, 6 May 2026 15:32:40 +0200 Subject: [PATCH 2/3] fixing gemini feedback --- lib/builder.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/builder.dart b/lib/builder.dart index 80325e0..c29176f 100644 --- a/lib/builder.dart +++ b/lib/builder.dart @@ -284,7 +284,7 @@ class _FirebaseFunctionsVisitor extends RecursiveAstVisitor { final target = node.target; final methodName = node.methodName.name; - if (_isFunctionsRunnerInvocation(methodName)) { + if (target == null && _isFunctionsRunnerInvocation(methodName)) { _visitRegistrationRunner(node); return; } From 720a68395a348411579d80108b433e3db31f697c Mon Sep 17 00:00:00 2001 From: Guillaume Bernos Date: Fri, 29 May 2026 09:25:01 +0200 Subject: [PATCH 3/3] simplify --- README.md | 6 ++++ lib/builder.dart | 35 ++++---------------- test/fixtures/dart_reference/bin/server.dart | 4 +-- 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index a31c30e..f17ba1e 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,12 @@ void main(List args) { } ``` +> [!IMPORTANT] +> Register your functions directly inside the `runFunctions` (or `fireUp`) +> callback. The build-time manifest generator only discovers registrations made +> within that callback, so functions registered from helper methods called by the +> callback will run locally but won't be deployed. + ## Status: Experimental This package provides a Dart implementation of Firebase Cloud Functions. Only HTTPS triggers are currently supported in production. Other trigger types are experimental and have [varying levels of support](doc/triggers.md). diff --git a/lib/builder.dart b/lib/builder.dart index 3b2ac61..7fc5d5c 100644 --- a/lib/builder.dart +++ b/lib/builder.dart @@ -301,15 +301,10 @@ class _FirebaseFunctionsVisitor extends RecursiveAstVisitor { /// firebase.https.onRequest(name: 'fn', options: opts, ...); final Map _variableToOptionsExpr = {}; - /// Top-level function declarations, used to follow helpers that are invoked - /// from the `runFunctions`/`fireUp` registration callback. - final Map _topLevelFunctions = {}; - - final Set _visitedRegistrationHelpers = {}; - - var _registrationDepth = 0; - - bool get _isCollectingRegistrations => _registrationDepth > 0; + /// Whether the visitor is currently inside the `runFunctions`/`fireUp` + /// registration callback. Only registrations reached while this is true are + /// discovered, so functions must be registered directly in the callback. + var _isCollectingRegistrations = false; @override void visitMethodInvocation(MethodInvocation node) { @@ -337,8 +332,6 @@ class _FirebaseFunctionsVisitor extends RecursiveAstVisitor { // Check for parameter definitions (top-level function calls with no target) if (_isParamDefinition(methodName)) { _extractParameterFromMethod(node, methodName); - } else if (_isCollectingRegistrations) { - _visitRegistrationHelper(methodName); } } @@ -372,8 +365,6 @@ class _FirebaseFunctionsVisitor extends RecursiveAstVisitor { for (final declaration in node.declarations) { if (declaration is TopLevelVariableDeclaration) { _trackOptionsVariables(declaration.variables); - } else if (declaration is FunctionDeclaration) { - _topLevelFunctions[declaration.name.lexeme] = declaration; } } super.visitCompilationUnit(node); @@ -398,30 +389,18 @@ class _FirebaseFunctionsVisitor extends RecursiveAstVisitor { switch (expression) { case FunctionExpression(:final body): _visitCollectingRegistrations(() => body.accept(this)); - case SimpleIdentifier(:final name): - _visitRegistrationHelper(name); case ParenthesizedExpression(:final expression): _visitRegistrationExpression(expression); } } - void _visitRegistrationHelper(String name) { - if (!_visitedRegistrationHelpers.add(name)) return; - - final declaration = _topLevelFunctions[name]; - if (declaration == null) return; - - _visitCollectingRegistrations(() { - declaration.functionExpression.body.accept(this); - }); - } - void _visitCollectingRegistrations(void Function() visit) { - _registrationDepth++; + final previous = _isCollectingRegistrations; + _isCollectingRegistrations = true; try { visit(); } finally { - _registrationDepth--; + _isCollectingRegistrations = previous; } } diff --git a/test/fixtures/dart_reference/bin/server.dart b/test/fixtures/dart_reference/bin/server.dart index 13bb889..5a27c0f 100644 --- a/test/fixtures/dart_reference/bin/server.dart +++ b/test/fixtures/dart_reference/bin/server.dart @@ -787,8 +787,8 @@ void main(List args) async { }); } -/// This helper is intentionally not called from [main]. The manifest builder -/// should only discover registrations reached during `runFunctions` setup. +/// Registrations made outside the `runFunctions`/`fireUp` callback are not +/// discovered: functions must be registered directly inside the callback. // ignore: unreachable_from_main void unregisteredHelper(Firebase firebase) { firebase.https.onRequest(