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

jakarta.inject support #2058

Open
cogman opened this issue Aug 20, 2020 · 46 comments
Open

jakarta.inject support #2058

cogman opened this issue Aug 20, 2020 · 46 comments

Comments

@cogman
Copy link

cogman commented Aug 20, 2020

Jakarta is coming to cause headaches for all :)

It would be nice if Dagger could support processing Jakarta inject annotations as well as javax inject annotations. That would prevent us from potentially having to write a cross injection system constructor like

@Inject
@jakarta.inject.Inject

Fortunately, they don't share a package and it looks like jakarta pretty much only changed the package name.

@bcorso
Copy link

bcorso commented Aug 20, 2020

I don't know what Jakarta is , but do you know why it is defining its own jsr330 annotations rather than using the standard javax.inject.* annotations?

Have you tried filing a bug with them to switch?

@tbroyer
Copy link

tbroyer commented Aug 20, 2020

@bcorso This is the new home for Java EE! Oracle killed javax.* and the Java EE name and forced Eclipse Jakarta to rename to jakarta.* and Jakarta EE (also applies to javax.servlet et al.)
This is a copyright and Oracle-bullshit issue, there's no way backwards 🤷‍♂️

@bcorso
Copy link

bcorso commented Aug 20, 2020

Oh... wow. Okay, yeah looks like we'll have to support all of jakarta.inject then.

@twillouer
Copy link

Hi, with Jersey 3.0 it will be mandatory I think :)

@overheadhunter
Copy link

Just a thought:

Instead of supporting both, javax.inject and jakarta.inject, would switching to solely supporting jakarta.inject be an option? For downstream projects this means s/import javax.inject/import jakarta.inject/g. Nothing particularly difficult, but still technically an API-breaking change.

If this was acceptable, implementing this change wouldn't increase the complexity in Dagger.

@Chang-Eric
Copy link
Member

Thanks for the idea, but for a large breaking change like that we'd have to support both at the same time for some (long) period of time to let people incrementally migrate.

@overheadhunter
Copy link

overheadhunter commented Jul 8, 2021

I just learned from the discussions in this PR, that jakarta.inject before version 2.0.0 still uses the old package name. So in fact this would be no breaking change when simply changing the dependency to jakarta.inject:jakarta.inject-api:1.0.3. These are these consequences:

downstream project depends only on Dagger depends on Dagger and javax.inject
uses module path ✅ Can now use reliable Automatic-Module-Name ⚠️ Split package
uses classpath ✅ No change ✅ Classes from both jars are identical by design, classloader may choose arbitrarily

The split package is only a virtual problem, since projects relying on the module path will avoid any such configuration. Nevertheless it should be documented.

It is even possible to have jakarta.inject:jakarta.inject-api:1.0.3 and jakarta.inject:jakarta.inject-api:2.0.0 on the module path at the same time, since the former uses java.inject (sic! as per JEP 200) and the latter jakarta.inject automatic module names.

@overheadhunter
Copy link

Just to confirm my previous suggestion: I just replaced javax.inject with jakarta.inject-api version 1.0.3 via dependency management in a multi-module project structure. No changes required. Works flawlessly on both modular and legacy projects.

Can you please consider this as a preliminary solution, until a "real" switch to jakarta namespace can happen eventually?

@Chang-Eric
Copy link
Member

Just to confirm your goals with this, this is so that you can publish a library with a named module that doesn't rely on the filename for the javax dependency since the jakarta dep provides an automatic module name?

If we switched Dagger's dependency to jakarta 1.0.3 for the javax annotations, I'm concerned this might pose a problem when people start migrating since if you need jakarta 2.0.0 for the jakarta annotations, I think Gradle's dependency resolution will just choose the 2.0.0 version and then the old usages of javax will break. I know you said above it is possible to have both 1.0.3 and 2.0.0 on the classpath and module path, but I feel like them being different versions of the same artifact is going to make that complicated for some build systems.

@overheadhunter
Copy link

Just to confirm your goals with this, this is so that you can publish a library with a named module that doesn't rely on the filename for the javax dependency since the jakarta dep provides an automatic module name?

Correct. In fact it has to be exactly 1.0.3. Older versions use a wrong module name, newer versions require you to change your imports.

If we switched Dagger's dependency to jakarta 1.0.3 for the javax annotations, I'm concerned this might pose a problem when people start migrating since if you need jakarta 2.0.0 for the jakarta annotations, I think Gradle's dependency resolution will just choose the 2.0.0 version and then the old usages of javax will break. I know you said above it is possible to have both 1.0.3 and 2.0.0 on the classpath and module path, but I feel like them being different versions of the same artifact is going to make that complicated for some build systems.

You're right, I don't know any build tool that allows having two versions of the same dependency simultaneously... 😔

@tbroyer
Copy link

tbroyer commented Jul 20, 2021

Having mixes of javax. and jakarta. artifacts is already a reality, and for some people mixes of javax. andjakarta. packages too (to a lesser extent), so having to manage your dependencies to select the appropriate dependency is already something we have to do (I suspect many people live with the mix of artifacts and duplicated classes).

So, we're getting off topic here as the original issue was about the packages, and this should probably be discussed in #2636 instead; but choosing jakarta.inject artifacts would:

  • be a "breaking" change anyway (could cause duplicate classes in downstream projects, from javax.inject ad jakarta.inject artifacts)
  • allow publishing artifacts that depend on Dagger and are JPMS modules, without having to manage dependencies to switch javax.inject to jakarta.inject in that project.

My opinion: I think I'd prefer that people manage their dependencies to replace javax.inject with jakarta.inject when they want to publish JPMS modules that depend on Dagger. This would put the burden on those projects and projects that use them, rather than every single project that uses Dagger.

<dependencies>
  <dependency>
    <groupId>com.google.dagger</groupId>
    <artifactId>dagger</artifactId>
    <version>2.x</version>
    <exclusions>
      <exclusion>
        <groupId>javax.inject</groupId>
        <artifactId>javax.inject</artifactId>
      </exclusion>
    </exclusions>
  </dependency>
  <dependency>
    <groupId>jakarta.inject</groupId>
    <artifactId>jakarta.inject-api</artifactId>
    <version>1.0.3</version>
  </dependency>
</dependencies>

@overheadhunter
Copy link

I think I'd prefer that people manage their dependencies to replace javax.inject with jakarta.inject when they want to publish JPMS modules that depend on Dagger.

While dependency management works great for applications (as I have previously suggested), it really isn't ideal for libraries. Since you explicitly mention publishing modules, I assume you have libraries in mind here.

If a library starts to manage its dependencies, the library's dependents will "unmanage" this by simply depending on dagger/javax.inject/jakarta.inject themselves (as long as the configuration is not replicated everywhere downstream), causing even more confusion.

Either way some projects will run into problems. So we're back to the original proposal:

  • either a breaking change in a major release
  • or as proposed by OP "support processing Jakarta inject annotations as well as javax inject annotations"

Obviously, the latter is the most flexible, allowing downstream projects to decide what annotations library to depend on.

@h908714124
Copy link

Do we really have to wait? dapper is a dagger clone that uses jakarta.inject and has a module descriptor.

@overheadhunter
Copy link

To gain some traction: So far we basically decided that changing dependencies just now is not an option.
But supporting both namespaces would allow users to decide what annotations to use.

So: What needs to be done in order to support both annotations?

@Chang-Eric
Copy link
Member

I think we can support both annotations, but possibly an area of concern will be dealing with Provider and backwards compatibility with libraries that have already compiled factories. Luckily, I think we might be able to just add it to the Factory interface (https://github.com/google/dagger/blob/master/java/dagger/internal/Factory.java) we already have and it may work but I will need to test it.

Note that this will mean Dagger is going to have both dependencies at the same time, not one or the other, and it will be in our runtime dependency. We will also have to be in this position for the foreseeable future. This would let library code avoid a direct dependency on javax, but the transitive dependency would still remain.

@overheadhunter I've kind of forgotten the context around the issues with the module system and javax, but I'm hoping being able to avoid the direct dependency will be enough?

@overheadhunter
Copy link

overheadhunter commented Oct 22, 2021

Note that this will mean Dagger is going to have both dependencies at the same time, not one or the other, and it will be in our runtime dependency.

No, sadly this would not solve the problem. As long as javax.inject doesn't get updated, it must be eradicated from the full dependency graph.

Still depending on any lib in dagger-compiler is fine. dagger, on the other side, should ideally check whether one or the other class is available dynamically. In other words: Make both javax.inject and jakarta.inject what Maven calls an optional dependency (1, 2) and let downstream projects provide one or the other (or both) as direct dependencies.

No idea how these kinds of dependencies works in Bazel, though. In Gradle it would be implementation rather than api called "capabilities", I guess.

@brettkail-wk
Copy link

brettkail-wk commented Oct 22, 2021

dagger, on the other side, should ideally check whether one or the other class is available dynamically. In other words: Make both javax.inject and jakarta.inject what Maven calls an optional dependency (1, 2) and let downstream projects provide one or the other (or both) as direct dependencies.

@overheadhunter Like @Chang-Eric says, it's not so simple because generated code and the Dagger runtime both reference javax.inject.Provider. The generated code issue could be solved with a processor option to choose javax vs jakarta. The Dagger runtime is harder, so my best idea is a new dagger.internal.jakarta package with a copy of all of the dagger.internal classes. Consumers probably don't want both sets of classes at runtime, so my best idea is to create a separate dagger-jakarta artifact for those users. However, having the same non-dagger.internal packages in multiple artifacts is messy for consumers (see Guava's -android and -jre), so perhaps create a dagger-core artifact with the shared pieces and dagger-internal-javax with dagger.internal, change dagger to only contain dependencies on those two and contain no classes, and then have dagger-jakarta similarly only depend only on dagger-core and dagger-internal-jakarta and contain no classes.

@overheadhunter
Copy link

However, having the same non-dagger.internal packages in multiple artifacts is messy for consumers (see Guava's -android and -jre), so perhaps create a dagger-core artifact with the shared pieces and dagger-internal-javax with dagger.internal, change dagger to only contain dependencies on those two and contain no classes, and then have dagger-jakarta similarly only depend only on dagger-core and dagger-internal-jakarta and contain no classes.

Probably a good idea to split up the artifacts. In fact it is hard to ignore that Dagger has a strong focus on Android lately (or always?), which adds lots of classes (like Hilt) that are otherwise irrelevant.

A little more modularity would certainly be beneficial, however splitting up artifacts deserves its own issue, I guess?

@Chang-Eric
Copy link
Member

If we split our artifacts, I think that could cause issues for libraries that use Dagger. Especially if a library has to inject a Provider, you could easily get into conflicts where one library uses javax and one library uses jakarta. Even if your project doesn't care to have both runtime dependencies, now there's a conflict because you have to choose one at the Dagger core level (as opposed to our generated factories supporting both). You couldn't pull in both Dagger artifacts because they would conflict. This forces every library to also split artifacts to avoid this issue. This is of course what those libraries would have to do to fully support people removing javax from dependencies entirely, but it escalates the issue from one of extra dependencies (and maybe a JPMS issue?) into a fully breaking one.

@overheadhunter

As long as javax.inject doesn't get updated, it must be eradicated from the full dependency graph.

Taking a step back, this sounds to me like possibly a javax problem to get updated. So far looking at the options to work around this issue in Dagger do not look good. Can you clarify what issues arise if javax is kept in the dependency graph with jakarta?

Also, to clarify, Hilt should already be separated into its own set of artifacts that do not affect the core Dagger artifacts. If you find a place where this is not the case though, please let us know (though maybe filed as a separate issue).

@brettkail-wk
Copy link

You couldn't pull in both Dagger artifacts because they would conflict.

@Chang-Eric Can you explain what you mean by "conflict"? My proposal for splitting the artifacts/packages was specifically to allow both dagger and dagger-jakarta to be included at the same time.

This forces every library to also split artifacts to avoid this issue.

This seems to already be happening in practice; main development of Jetty, Jersey, etc. only support jakarta.. For the services I work on, the only reason javax. is included is for Dagger.

Can you clarify what issues arise if javax is kept in the dependency graph with jakarta?

Primarily developer confusion around which one to import. (It also adds an extra very small JAR everywhere.)

@Chang-Eric
Copy link
Member

@brettkail-wk Ah, I think I read your suggestion too fast and missed that you were saying to split the packages as well. With that said, I think it could get really complicated. I think for example we'd actually have to fork our code twice, so there'd be a javax, jakarta, and javax-and-jakarta package and artifacts. We'd likely have to also create wrapper code to translate a javax Factory into a javax-and-jakarta Factory since the choices of which Provider type we need will depend on the injection sites, not the binding definition where the Factory is generated. It is probably doable...but definitely will be a significant cost and maintenance burden and also a cost that users will have to bear as well since they will have to manage which Dagger artifacts they use.

Primarily developer confusion around which one to import. (It also adds an extra very small JAR everywhere.)

While this is kind of unfortunate, I feel like there will also likely be a similar confusion with the package/artifact splitting route as well. Making dagger.internal.Factory extend both is still looking to be the best solution to me since it doesn't leak out beyond that class at all, so for the most part, users can just choose which they need to use (many will need to choose javax to interface with other code that hasn't been updated yet) and use it.

@brettkail-wk
Copy link

I think for example we'd actually have to fork our code

I had thought it would be easier to auto-generate the secondary package(s) when building Dagger itself, which would reduce the maintenance burden.

We'd likely have to also create wrapper code to translate a javax Factory into a javax-and-jakarta Factory

To be sure, this doesn't seem strictly necessary since it seems Dagger could require the injection site to use the same Provider type as the factory. This is surely less developer-friendly, but it would be simpler to implement and would only affect code that is mid-transition to jakarta.

Making dagger.internal.Factory extend both is still looking to be the best solution

It seems unfortunate to choose a solution without having some kind of plan for eventually removing javax..

Anyway, it's clear you understand my suggestion (even if you prefer another), so I'll go back to watching :-).

@overheadhunter
Copy link

overheadhunter commented Oct 26, 2021

Taking a step back, this sounds to me like possibly a javax problem to get updated.

Indeed, this is a problem in javax.inject, not in Dagger. However, javax.inject is no longer maintained with the last change being 12 years ago. In the meantime, with the transition from JEE to Jakarta, Eclipse is responsible for maintaining it and in fact they did recently publish fixes for both, jakarta.inject (jar major versions 2.x+) namespace as well as for javax.inject (jar major version 1.x). However, despite being 100% identical to the former javax.inject jar, they use different Maven coordinates.

So far looking at the options to work around this issue in Dagger do not look good. Can you clarify what issues arise if javax is kept in the dependency graph with jakarta?

This is not merely about developer confusion. The problem with the 12-year-old javax.inject jar is that it does not provide an Automatic Module Name. In other words: Beginning with Java 9, its automatic module name is guessed from the jar file name, which is considered unreliable and results in a warning:

[WARNING] *******************************************************************************************************************************************************************************
[WARNING] * Required filename-based automodules detected: [javax.inject-1.jar]. Please don't publish this project to a public artifact repository! *
[WARNING] *******************************************************************************************************************************************************************************

Adding a new dependency to jakarta.inject will not solve it. In fact there is no benefit from adding jakarta.inject at all. It is just to replace a jar that can not be used in downstream libraries that aim to be published for Java 9+.

There are further issues related to tools like jlink not being able to calculate the module graph as long as Dagger doesn't include a module description, but these problems are irrelevant as long as the root cause isn't solved.

@Chang-Eric
Copy link
Member

@overheadhunter 2.x with jakarta.inject

@overheadhunter
Copy link

I see, sounds good!

@brettkail-wk
Copy link

@Chang-Eric The generated classes also reference javax.inject.Provider directly. Would there be a corresponding 2.40-jakarta version of dagger-compiler to generate jakarta.inject instead, or how would that work?

@Chang-Eric
Copy link
Member

@brettkail-wk Yes, we'll update dagger-compiler to make the generated factories just handle everything via the dagger.internal.Factory type (as well as handle the other annotations). After this change, within the runtime, I would expect there to be no references to either Provider type in Dagger or Dagger-generated code except via the dagger.internal.Factory class. That's the plan at least anyway, we'll see what actually happens when we try doing it.

copybara-service bot pushed a commit that referenced this issue Jan 26, 2022
…nd Singleton. This does not get rid of the runtime dependency nor does it support Provider yet.

Issue #2058.

RELNOTES=Support Jakarta versions of Inject, Scope, Qualifier, and Singleton
PiperOrigin-RevId: 418678754
copybara-service bot pushed a commit that referenced this issue Jan 26, 2022
…nd Singleton. This does not get rid of the runtime dependency nor does it support Provider yet.

Issue #2058.

RELNOTES=Support Jakarta versions of Inject, Scope, Qualifier, and Singleton
PiperOrigin-RevId: 424427709
@ThomasVitale
Copy link

Is there any update regarding this issue?

@Chang-Eric
Copy link
Member

Unfortunately no. I did part of the changes so that you can use Jakarta types besides Provider, but the Provider changes would be more involved and we just haven't been able to prioritize that work at this time.

@ThomasVitale
Copy link

@Chang-Eric thanks a lot for your answer!

@h908714124
Copy link

simple-component is another alternative for those who cannot wait.

@r0main
Copy link

r0main commented Apr 4, 2023

Hi, same question as @ThomasVitale :-) Is there any news on the subject ?

I have code that is shared between a Spring Boot app and an AWS Lambda. The shared code depends on javax.*. My Spring Boot app uses Spring as DI (obviously) and my AWS Lambda uses Dagger as DI (as Dagger is faster to startup).
I would like to upgrade Spring Boot to version 3. So I nee to migrate the shared code to Jakarta EE 9 (it mandatory when using Spring Boot 3). And you see me coming... If I upgrade the shared code to Jakarta EE 9, I need a Dagger version compatible with it to keep my AWS Lambda working :-)

Thank you in advance for the news :-)

Best regards !

@Chang-Eric
Copy link
Member

I'm going to try to add Provider support soon, which should cover all of the symbols for Jakarta. Cutting the dep to javax to fix the JPMS issues will likely take more time though.

@m-presnov
Copy link

m-presnov commented Nov 24, 2023

Any news about support jakarta.inject.Provider in dagger-compiler?

@Chang-Eric
Copy link
Member

I actually have a change ready for the first step of adding our own internal Dagger Provider type. I'm hoping to get it out soon. We ended up not being able to just fix it with dagger.internal.Factory because unsurprisingly a lot of our code references javax.inject.Provider directly and that all needs to be replaced. It also breaks the signature of a lot of our runtime methods, so in order to maintain backwards compatibility, there's extra code to detect and handle different backwards compatibility situations between libraries when we generate code.

Frankly, it's been a huge pain even though it seems like a simple rename, which is why this is taking so long. Also, since this change has the potential to break a lot of things, we're trying to isolate it into its own release which is causing some delays too. Hopefully this first step should be out soon. It won't let you break the javax dependency yet, but once that is out and there aren't hopefully any issues, we should hopefully be able to then release an artifact that has our dagger.internal.Provider only extend the Jakarta version of Provider. Note that even with all that, there's still going to be complications with our SPI and how we're going to break some external plugins if we change the binding keys that have Provider in them to the Jakarta type (e.g. some of our Map<K, Provider<V>> bindings), so there's still something to figure out there too.

@jeffalder
Copy link

@Chang-Eric How can we help? Do you have things that need coding? Testing? I'm very interested in helping the team figure this out. Clarifying "this converts to Jakarta, this does not" has been a pain point for folks I support that are using Jakarta and Dagger 2, so I'm motivated.

@Chang-Eric
Copy link
Member

I just submitted a change in 75d3cbc to add the new dagger.internal.Provider. This won't change anything with Jakarta deps yet, but it is the next step. Hopefully we'll release this soon, so people can start building with this, and then I'll do a change to allow using the Jakarta version in user code. We'll also have to do a change that will break compatibility across versions because unfortunately some of our factories are generated returning the javax.inject.Provider directly, but hopefully that will only be necessary once we actually try to drop the javax dependency.

One way you can help is if you want to try out the current change at head to make sure there isn't any issue. https://dagger.dev/dev-guide/versions has info on how to depend on the HEAD-SNAPSHOT.

@jeffalder
Copy link

I ran a quick test on one of my repos with HEAD-SNAPSHOT and it was fine (as I expected). We don't generally do anything too fancy with Dagger, we don't integrate directly with the Provider classes, and we rarely (if ever?) even share Dagger generated code. I kind of expect this effort to be mostly trivial for us.

@r0main
Copy link

r0main commented Dec 15, 2023

Hi,

We have a small project using Dagger. We also tested the head-snapshot version and it worked fine ! Thank you for the work !

I have a question : I had to replace javax package by jakarta package in my code. That was absolutely expected. But in the generated code by Dagger, in *_Factory classes, I saw that there is style javax import, is it normal ? Is it releated to your last message @Chang-Eric ? I admit I didn't understand everything but you're talking about factories and javax :-)

Hope this test can help even if it's small !

@dnouls
Copy link

dnouls commented Mar 26, 2024

Any idea if support for Jakarta API's is going to be supported with Dagger? We need to migrate to newer versions of software for security and maintainability reasons, so the problem with Jakarta is starting to become quite a big issue.

@jeffalder
Copy link

Any idea if support for Jakarta API's is going to be supported with Dagger? We need to migrate to newer versions of software for security and maintainability reasons, so the problem with Jakarta is starting to become quite a big issue.

@dnouls My experience is that I was able to update my server (e.g., Netty) to Jakarta, but retain the javax.inject API for use with Dagger. In our case, the two were not coupled together. Are you using anything that would force them to use the same packages?

@Chang-Eric
Copy link
Member

Any idea if support for Jakarta API's is going to be supported with Dagger?

Jakarta APIs like @jakarta.inject.Inject should currently work except for using jakarta.inject.Provider. I'm currently actually working on that (usually a bit each week as I have time), but am running into lots of issues internally that keep blocking this. Unfortunately I don't have an ETA, but this actually is being worked on. It is just slow going as new issues pop up and I don't always have a lot time to dedicate to it.

copybara-service bot pushed a commit that referenced this issue May 17, 2024
Issue #2058.

RELNOTES=Add a jakarta.inject.Provider runtime dependency in preparation for supporting Jakarta Providers
PiperOrigin-RevId: 633750268
copybara-service bot pushed a commit that referenced this issue May 17, 2024
Issue #2058.

RELNOTES=Add a jakarta.inject.Provider runtime dependency in preparation for supporting Jakarta Providers
PiperOrigin-RevId: 633750268
copybara-service bot pushed a commit that referenced this issue May 17, 2024
Issue #2058.

RELNOTES=Add a jakarta.inject.Provider runtime dependency in preparation for supporting Jakarta Providers
PiperOrigin-RevId: 634896178
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests