Dependency injection #447
Dependency injection #447
Conversation
Allow the consumer to *optionally* supply an actual templateAdapter instance to prevent Rendr from performing a dynamic require.
Allow the calling code to *optionally* supply the ClientRouter class so Rendr doesn't attempt to require it.
jmerrifield
force-pushed
the
dependency-injection
branch
from
85bf359 to
1ae60e7
Compare
jmerrifield
force-pushed
the
dependency-injection
branch
from
de3d1b4 to
f932400
Compare
|
👯 |
There was a problem hiding this comment.
require in the browser? Looks like something very specific to browserify.
There was a problem hiding this comment.
it was already doing this on line 13? not sure how it's terribly different from the original functionality there. (Totally agree with you though.. 😞 )
Any suggestions to fix it up? my only thought is this.modelUtils._requireAMD(defaultRouterModule) instead.
There was a problem hiding this comment.
Before it had "hardcoded" default router path, so r.js was able to augment it and with this change it makes it invisible for r.js.
There was a problem hiding this comment.
Btw, looks like assign path to a variable is not essential to this change. So we might revert that particular line to the previous state. While we're thinking on better implementation.
In general IoC should make things better, but here it went sideways :)
There was a problem hiding this comment.
yeah, balls. sorry about that guys. is the requirejs example updated so i can use it as a test bed for this stuff?
There was a problem hiding this comment.
@saponifi3d we have this PR rendrjs/rendr-examples#15 to make it work with Rendr 1.0.1
There was a problem hiding this comment.
Hmm, that doesn't seem to fix it for me, i'm getting an error on this line https://github.com/rendrjs/rendr/blob/master/shared/base/router.js#L184 - it looks like realAction is a string not the action that is expected.
In the meantime i'm going to revert the commit that caused the breakage and push out 1.0.4 (i think it's that version?) so it's in a working state for everyone.
There was a problem hiding this comment.
@saponifi3d just tested the sample app (05_requirejs) and it seem to be working with the latest version of Rendr 1.0.2 but we still see the require issue.
There was a problem hiding this comment.
Yeah, I haven't merged / released those changes yet for the require. I just did a fresh install and tried the example with 1.0.1 and it didn't work. Its breaking on the real action of the controller on a client-side route. Until there's a working exple there it will be hard for me to make a proper fix.
5 participants
I'd like to be able to inject pre-
required modules fortemplateAdapterandClientRouterso that I don't have toexposethem through Browserify config, in order for Rendr to require them for me. These optional attribute values should enable that use case without breaking any existing usages.