Skip to content

Use @Singleton again #118

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

Merged
merged 20 commits into from
Jan 8, 2023
Merged

Use @Singleton again #118

merged 20 commits into from
Jan 8, 2023

Conversation

SentryMan
Copy link
Collaborator

@SentryMan SentryMan commented Jan 6, 2023

  • uses @component only if Avaje is on the classpath
  • uses Jakarta/javax @singleton depending on classpath
  • uses Jakarta @singleton by default if both jakarta and javax on classpath
  • adds a compiler arg to force javax

@rbygrave
Copy link
Contributor

rbygrave commented Jan 6, 2023

Yeah, I don't think this is the best approach. We ideally have 4 cases to support.

  • using avaje-inject Jakarta (ignoring javax)
  • using avaje-inject javax (ignoring Jakarta)
  • using some other DI via Jakarta
  • using some other DI via javax

As an alternative approach, it can detect avaje-inject and then failing that detect Jakarta or javax.

@SentryMan
Copy link
Collaborator Author

SentryMan commented Jan 6, 2023

I can easily add the avaje detection and use @component for cases 1 and 2. But for scenarios 3 and 4, I think defaulting to jakarta unless specified is the way to go. Consider the scenario where javax/jakarta just happens to be on the classpath because of a transitive dependency. How should the processor proceed? A flag makes it simple.

@SentryMan
Copy link
Collaborator Author

@rbygrave how about this?

Will use avaje if it's there. Otherwise will use either jakarta/javax on the classpath. If both are on the classpath, will default to jakarta, unless overridden by the flag.

Copy link
Contributor

@rbygrave rbygrave left a comment

Choose a reason for hiding this comment

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

Yup, excellent.

@rbygrave rbygrave merged commit 798ca73 into avaje:master Jan 8, 2023
@rbygrave rbygrave added this to the 1.21 milestone Jan 8, 2023
@rbygrave rbygrave added the enhancement New feature or request label Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants