fmgasparino / google-gin

Automatically exported from code.google.com/p/google-gin
Apache License 2.0
0 stars 0 forks source link

GWT.create() not called on interfaces in bind() #13

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Create an interface.
2. Create a concrete implementation of said interface that @Inject Constants, 
Messages and/or 
RemoteServices.
3. Bind the concrete implementation to the interface in a AbstractGinModule.

What is the expected output? What do you see instead?
The expected output is GWT.create() being called on the Constants, Messages 
and/or 
RemoteServices. Instead, something like the following error message appears:

[ERROR] Guice creation errors:

1) No implementation for com.test.client.ViewConstants was bound.
 while locating com.test.client.ViewConstants
   for parameter 0 at com.test.client.ViewImpl.<init>(ViewImpl.java:11)
 at com.google.gwt.inject.rebind.adapter.BinderAdapter.bind(BinderAdapter.java:33)

What version of the product are you using? On what operating system?
SVN HEAD (revision 60). Mac OS X Leopard.

Please provide any additional information below.

I've patched a test case to show the error. The patch is attached below.

Original issue reported on code.google.com by arthur.k...@gmail.com on 22 Dec 2008 at 7:57

Attachments:

GoogleCodeExporter commented 9 years ago
I can reproduce with the attached test. Investigating.

Original comment by bstoler+code@google.com on 22 Dec 2008 at 8:37

GoogleCodeExporter commented 9 years ago
OK, I understand what is going on now.

Before generating the output, we run the modules through Guice at TOOL stage to
verify that they are consistent. The problem is that the synthetic GWT.create
bindings are never exposed to Guice -- Gin synthesizes them itself outside of 
the
Guice modules.

So the implicit bindings work when the class that needs them does not have an
explicit binding in Guice because Guice doesn't notice that whole tree. But as 
this
patch shows, if Guice knows about class X and X needs MyConstants, Guice thinks 
that
MyConstants (an interface) is not bound.

We can't actually usefully explain to Guice how to create objects via 
GWT.create, but
luckily we don't have to -- we just need to tell Guice that something can create
them. So I think we can fix this by having createImplicitBinding build up a 
list of
Guice Providers to tell Guice that the implicitly bound things are in fact 
creatable.

I'll try to take a look later today.

Original comment by bstoler+code@google.com on 22 Dec 2008 at 9:00

GoogleCodeExporter commented 9 years ago
Thanks Brian!

I forgot to mention (for others reading the issue) that there is a work around. 
Just create Provider yourself and 
manually bind the interface to that Provider that calls GWT.create().

Original comment by arthur.k...@gmail.com on 22 Dec 2008 at 9:11

GoogleCodeExporter commented 9 years ago
Just ran into the same issue -- before seeing this bug I also created a failing 
test
(attached).

I'll add that I'm relying on the GWT.create fallback to inject ImageBundle and
ServiceAsync objects.  No big deal to write a custom provider, but it sure is 
sweet
when it works magically =)

Original comment by philcoak...@gmail.com on 23 Dec 2008 at 7:19

Attachments:

GoogleCodeExporter commented 9 years ago
So, I spent a bunch of time trying to fix this by telling Guice about all of the
implicit bindings we have. It didn't go so well. I'll try again after I submit 
my
current pending changes.

If we can't make this work, we might need to remove the use of Guice to validate
module correctness. However, this would reopen issue 9 and issue 10 (and other
unknown issues), which would be bad.

Original comment by bstoler+code@google.com on 28 Dec 2008 at 12:54

GoogleCodeExporter commented 9 years ago
I have this working. I need to clean up the patch and then I can send it out.

Original comment by bstoler+code@google.com on 31 Dec 2008 at 9:56

GoogleCodeExporter commented 9 years ago
Sweet! I look forward to it, but enjoy your holiday :P. Happy New Year.

Original comment by arthur.k...@gmail.com on 31 Dec 2008 at 10:07

GoogleCodeExporter commented 9 years ago
Code review : http://codereview.appspot.com/11861

Fixed in r73.

Original comment by bstoler+code@google.com on 2 Jan 2009 at 6:33

GoogleCodeExporter commented 9 years ago
Hi Brian, I didn't get a chance to look at this till now, but my original user 
case now works. Thanks for the great 
work!

Original comment by arthur.k...@gmail.com on 8 Jan 2009 at 9:06