From 243780787329dbfb9a889658154be212bd64e271 Mon Sep 17 00:00:00 2001
From: Sung Yoon Whang <sungyoon@uber.com>
Date: Fri, 19 Aug 2022 15:20:42 -0700
Subject: [PATCH] Fix Invoke order when fx.Module is used (#925)

* Fix Invoke order when fx.Module is used

Currently, fx.Module's invokes are run after all the invokes provided
at the parent level are invoked.

This makes it difficult for module consumers to control the precise
invoke orders.

This changes the invoke order to run all the invokes in the child
module before running any of the invokes in the parent module, and
clarify that order in the documentation for Invoke.

Solves #918.
Refs GO-1591.

* fix indentation in code sample
---
 invoke.go      | 15 +++++++++++++++
 module.go      | 11 +++++++----
 module_test.go | 30 ++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/invoke.go b/invoke.go
index d7ce9317b..523aef83b 100644
--- a/invoke.go
+++ b/invoke.go
@@ -38,6 +38,21 @@ import (
 // was successful.
 // All other returned values are discarded.
 //
+// Invokes registered in [Module]s are run before the ones registered at the
+// scope of the parent. Invokes within the same Module is run in the order
+// they were provided. For example,
+//
+//	fx.New(
+//		fx.Invoke(func3),
+//		fx.Module("someModule",
+//			fx.Invoke(func1),
+//			fx.Invoke(func2),
+//		),
+//		fx.Invoke(func4),
+//	)
+//
+// invokes func1, func2, func3, func4 in that order.
+//
 // Typically, invoked functions take a handful of high-level objects (whose
 // constructors depend on lower-level objects) and introduce them to each
 // other. This kick-starts the application by forcing it to instantiate a
diff --git a/module.go b/module.go
index 4b5aaa7f3..c8b8acad0 100644
--- a/module.go
+++ b/module.go
@@ -40,6 +40,8 @@ type container interface {
 }
 
 // Module is a named group of zero or more fx.Options.
+// A Module creates a scope in which certain operations are taken
+// place. For more information, see [Decorate], [Replace], or [Invoke].
 func Module(name string, opts ...Option) Option {
 	mo := moduleOption{
 		name:    name,
@@ -151,17 +153,18 @@ func (m *module) provide(p provide) {
 }
 
 func (m *module) executeInvokes() error {
-	for _, invoke := range m.invokes {
-		if err := m.executeInvoke(invoke); err != nil {
+	for _, m := range m.modules {
+		if err := m.executeInvokes(); err != nil {
 			return err
 		}
 	}
 
-	for _, m := range m.modules {
-		if err := m.executeInvokes(); err != nil {
+	for _, invoke := range m.invokes {
+		if err := m.executeInvoke(invoke); err != nil {
 			return err
 		}
 	}
+
 	return nil
 }
 
diff --git a/module_test.go b/module_test.go
index 8e00e892e..13be8be37 100644
--- a/module_test.go
+++ b/module_test.go
@@ -211,6 +211,36 @@ func TestModuleSuccess(t *testing.T) {
 
 		defer app.RequireStart().RequireStop()
 	})
+
+	t.Run("Invoke order in Modules", func(t *testing.T) {
+		t.Parallel()
+
+		type person struct {
+			age int
+		}
+
+		app := fxtest.New(t,
+			fx.Provide(func() *person {
+				return &person{
+					age: 1,
+				}
+			}),
+			fx.Invoke(func(p *person) {
+				assert.Equal(t, 2, p.age)
+				p.age += 1
+			}),
+			fx.Module("module",
+				fx.Invoke(func(p *person) {
+					assert.Equal(t, 1, p.age)
+					p.age += 1
+				}),
+			),
+			fx.Invoke(func(p *person) {
+				assert.Equal(t, 3, p.age)
+			}),
+		)
+		require.NoError(t, app.Err())
+	})
 }
 
 func TestModuleFailures(t *testing.T) {