Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Private Modules #1030

Closed
wants to merge 1 commit into from
Closed

Private Modules #1030

wants to merge 1 commit into from

Conversation

JacobOaks
Copy link
Contributor

Some users have been requesting that our private provides functionality provided by #995 be extended by allowing a way to privatize an entire module (Ref: #1020).

This change modifies fx.Private to implement fx.Option, allowing it to be passed to fx.App and fx.Module.
When passed into a module, the entire module will be marked as private, meaning all provides and supplies of that module and its inner modules will be marked as private.

fx.New(
	fx.Module("SubModule",
		fx.Private,
		fx.Supply(5),
		fx.Provide(func() string { return "b"}),
		fx.Invoke(a int, b str) {}, // Runs w/ a = 5, b = "b"
	),
	fx.Invoke(func(a int) {}), // This will fail
	fx.Invoke(func(b str) {}), // This will also fail
)

I opted for this API to stay consistent with the style of API we already allow for passing Private to Provide directly.

This change also extracts all private-related definitions and testing out into separate files, private.go and private_test.go.

Some users have been requesting that our private provides functionality
provided by uber-go#995 be extended by allowing a way to privatize an entire module.

This change modifies fx.Private to implement fx.Option,
allowing it to be passed to fx.App and fx.Module.
When passed into a module, the entire module will be marked as private,
meaning all provides and supplies of that module and its inner modules
will be marked as private.

```go
fx.New(
	fx.Module("SubModule",
		fx.Private,
		fx.Supply(5),
		fx.Provide(func() string { return "b"}),
		fx.Invoke(a int, b str) {}, // Runs w/ a = 5, b = "b"
	),
	fx.Invoke(func(a int) {}), // This will fail
	fx.Invoke(func(b str) {}), // This will also fail
)
```

I opted for this API to stay consistent with the style of API
we already allow for passing Private to Provide directly.

This change also extracts all private-related definitions and testing
out into separate files, `private.go` and `private_test.go`.
@codecov
Copy link

codecov bot commented Jan 23, 2023

Codecov Report

Merging #1030 (f126cf8) into master (5f5dc19) will decrease coverage by 0.19%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##           master    #1030      +/-   ##
==========================================
- Coverage   98.04%   97.86%   -0.19%     
==========================================
  Files          39       40       +1     
  Lines        1994     2010      +16     
==========================================
+ Hits         1955     1967      +12     
- Misses         31       34       +3     
- Partials        8        9       +1     
Impacted Files Coverage Δ
app.go 94.68% <0.00%> (-1.54%) ⬇️
fxevent/event.go 100.00% <ø> (ø)
private.go 50.00% <50.00%> (ø)
fxevent/console.go 100.00% <100.00%> (ø)
fxevent/zap.go 100.00% <100.00%> (ø)
module.go 100.00% <100.00%> (ø)
provide.go 100.00% <100.00%> (ø)
supply.go 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@JacobOaks JacobOaks closed this Jan 23, 2023
@JacobOaks
Copy link
Contributor Author

Closing for now, missed a couple key things

@abhinav
Copy link
Collaborator

abhinav commented Oct 28, 2024

@JacobOaks out of curiosity: do you remember what was missing here?

@JacobOaks
Copy link
Contributor Author

JacobOaks commented Oct 30, 2024

@JacobOaks out of curiosity: do you remember what was missing here?

I think we were unsure about the utility an entirely private module would provide. After all, an entirely private module, by definition, cannot contribute anything to an application's container. However, invokes within the private module would still run and cause the private module's dependencies to build - so I guess that's an exception. But that almost sort of feels like encouraging strange side-effect/hyrum's law-esque dependencies.

I think something closer to @jkanywhere's suggestion on #1020 may be better: it allows easily "privatizing" larger collections of options, while still allowing otherwise mostly private modules to expose key type(s) to other modules/the container.

@abhinav
Copy link
Collaborator

abhinav commented Oct 30, 2024

I think we were unsure about the utility an entirely private module would provide.

Ack, yeah. So the use case I was imagining was something like this.
I have an Fx module that supports one of something, e.g. spinning up an HTTP server.

fx.New(httpserver.Module).Run()

For reasons, I want to have two separate HTTP servers.
So I would be able to do something like this:

fx.New(
  fx.Module("main", fx.Private(httpserver.Module), /* handlers for main */),
  fx.Module("internal", fx.Private(httpserver.Module), /* handlers for internal */),
)

I hadn't fully thought through how private modules would "export" values outside the private module.
I'd originally imagined that Private just inverts the Fx default of exporting from subscopes, but in hindsight, I don't like that much.

I think something closer to @jkanywhere's suggestion on #1020 may be better

I like that! It's intuitively similar to internal/ directories in Go: only siblings and direct parent can use outputs of private modules.
There would be no way for a parent to export the private result to the rest of the graph, but IMO that's not an issue—if that's needed, then thing isn't private.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants