Skip to content

Attempt multi scope no deps #802

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

Closed
wants to merge 10 commits into from

Conversation

cbarlin
Copy link
Contributor

@cbarlin cbarlin commented Apr 12, 2025

An attempt at building off of @SentryMan's allRequiredModules method...

It doesn't work for modules that have dependencies though, and I couldn't easily see a way to automatically bail if that was the case. It does work for the blackbox-multi-scope though...

Sorry, wasn't sure where to put this attempt, and boiling down the changes I made to write in a ticket would probably be difficult.

@@ -102,7 +106,7 @@ void write() throws IOException {
private void writeRequiredModules() {

ProcessingContext.modules();
Set<String> requiredModules = new TreeSet<>();
List<String> requiredModules = new ArrayList<>();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not needed - I ran into an issue where it was complaining about missing required scopes, so was trying a few things until I made the change to have the scope auto-provide itself

import io.avaje.inject.InjectModule;
import jakarta.inject.Scope;

@Scope
@InjectModule(requires = {ModAModule.class, ModBModule.class})
@InjectModule(requires = {ModAScope.class, ModBScope.class})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These being modules or scopes is a hard one by the looks:

  • If they are modules, then we don't need to add to the autoProvides (although it works without this PR without doing that 🤔 )
  • If they are scopes, you can easily find the scopes they rely on

But then, either way, it (currently) falls down when you attempt to extract the module dependencies to build the method... So maybe having them as modules is better? Although then you may be trying to read something from the annotation that doesn't yet exist and I've ran into that before as a problem in another project...

Copy link
Collaborator

@SentryMan SentryMan Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so modules in the annotation doesn't fly because the module classes don't exist by the time the processor tries to read them

@SentryMan SentryMan force-pushed the attempt_multi_scope_no_deps branch from 830baa9 to 3fd05dc Compare April 12, 2025 16:56
@SentryMan SentryMan marked this pull request as ready for review April 12, 2025 16:57
@SentryMan SentryMan requested a review from rbygrave April 12, 2025 16:57
@SentryMan
Copy link
Collaborator

I've pulled your changes into #800

@SentryMan SentryMan closed this Apr 12, 2025
@cbarlin cbarlin deleted the attempt_multi_scope_no_deps branch April 12, 2025 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants