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

Circular Dependencies with cs plugin don't work well. #13

Open
hisayan opened this issue Dec 28, 2011 · 12 comments
Open

Circular Dependencies with cs plugin don't work well. #13

hisayan opened this issue Dec 28, 2011 · 12 comments

Comments

@hisayan
Copy link

hisayan commented Dec 28, 2011

ref http://requirejs.org/docs/api.html#circular and https://github.com/jrburke/requirejs/tree/master/tests

# onecs.coffee
define [ "require", "cs!twocs" ], (require, two) ->
  console.log two
  one = 
    size: "large"
    doSomething: ->
      require "cs!twocs"

  one
# twocs.coffee
define [ "require", "cs!onecs" ], (require, one) ->
  console.log one
  size: "small"
  color: "redtwo"
  doSomething: ->
    require("onecs").doSomething()
# circular-tests-cs.js
require({
        baseUrl: "./"
    },
    ["require", "cs!twocs"],
    function(require, two) {
        var args = two.doSomething();
        doh.register(
            "circular",
            [
                function circular(t) {
                    t.is("small", args.size);
                    t.is("redtwo", args.color);
                }
            ]
        );
        doh.run();
    }
);
#circular-cs.html
<!DOCTYPE html>
<html>
<head>
    <title>require.js: Circular Test</title>
    <script type="text/javascript" src="../require.js"></script>
    <script type="text/javascript" src="doh/runner.js"></script>
    <script type="text/javascript" src="doh/_browserRunner.js"></script>
    <script type="text/javascript" src="circular-tests-cs.js"></script>
</head>
<body>
    <h1>require.js: Circular Test</h1>
    <p>Check console for messages</p>
</body>
</html>

Uncaught Error: Load timeout for modules: cs!twocs cs!onecs

What should I do ?

my English is not good.

@hisayan hisayan closed this as completed Dec 28, 2011
@hisayan hisayan reopened this Dec 28, 2011
@jrburke
Copy link
Member

jrburke commented Jan 9, 2012

Sorry for the late comment:

For twocs.coffee if you do this instead, does it work (add the 'cs!' to the beginning of the dependency)?

require("cs!onecs").doSomething()

@hisayan
Copy link
Author

hisayan commented Jan 9, 2012

Thanks.

I tried it. but it has returned same message.

Uncaught Error: Load timeout for modules: cs!twocs cs!onecs

@jrburke
Copy link
Member

jrburke commented Jan 9, 2012

OK, thanks for checking. I'll take a look hopefully early this week. I expect it is more of a problem with how plugins work than a problem with a circular dependency inherent in the loader.

@BryanRJ
Copy link

BryanRJ commented Jan 17, 2012

I have tested with the latest require.js in master (as of today) and it still has problems with circular deps and plugins.

I do have a contradiction to anotherbrain's comment, though: in r.js running under node, eliminating the dependency from the array passed to define() and instead using a runtime require() causes the optimizing pass to complete successfully. Effectively, hiding the dependency breaks the circularity the optimizer sees and resolves the issue. However, this takes real care since unless some other code includes the file you do this way, r.js won't be aware of it and won't bundle it into the final file... I don't know what this does in the browser.

@hisayan
Copy link
Author

hisayan commented Jan 18, 2012

I want to use factory method pattern.
for example.

# animal.coffee
define [ "require" ], (require) ->
  class Animal =
    @type:  'animal'
    @create: (options) ->
      switch options.type 
        when 'dog' then new require('cs!dog') options
        when 'cat' then new require('cs!cat') options
        else throw new Error 'animal type "' + options.type + '" not supported.'
# dog.coffee
define ['require', 'cs!animal'], (require, Animal) ->
  class Dog extends Animal
    @type: 'dog'
    @initialize: (options) ->
      super.initialize options
      # do something

@dustinboston
Copy link

@jrburke I can confirm this is still an issue as of the latest stable release. Trying to come up with a fix but I'm not sure that I'll be able to resolve this one. Thanks James!

@dustinboston
Copy link

You know what, looking over the require docs I saw this little gem: "Circular dependencies are rare, and usually a sign that you might want to rethink the design." So, I'm probably a dumb-ass for ending up with a circular dependency and I will just resolve that issue. :-)

However, wondering if there is something that can be done to make it more obvious that there's a problem. Right now you get the generic timeout error. Maybe there should be a circular dependencies error to point out the fact that I'm being stupid?

@jrburke
Copy link
Member

jrburke commented Jun 12, 2012

@dustinboston I'm hoping to highlight circular dependencies with the (to be renamed) reqinfo library, which someone can drop in alongside require.js to get extra checks/dep graph. But so far it is vaporware.

With the 2.0 code structure, I can now see a way that I may be able to allow circular loader plugin dependencies (although not sure if I could break them), but that would need a code overhaul to do so, so likely a 2.1 or a 3.0 thing. But again, not sure if I could actually break the cycle correctly in that case. But for transpiler plugins, I can see it useful to have circular dependencies, and that may be a subclass of plugin that may be possible to break the cycle correctly.

@dustinboston
Copy link

@jrburke Thanks for looking into it. I'm still in favor of a "Dude, your code sucks, stop doing that!" error message though ;-) FWIW, my code is now better because I removed the circular dependencies. Just takes a little thought to re-architect something in a more straight-forward manner.

@omares
Copy link

omares commented Jun 14, 2012

@dustinboston How did you solve the circular dep?
I am using marionette+backbone+coffe script and have the issue that my app needs to know the view, and vice versa.

@dustinboston
Copy link

I hate to bump this thread but I did want to reply to @omares. The answer is lots of pub/sub. Just had another case similar to @anotherbrain (using a factory method). Instead of including the factory from the factory I created a single place in the app that creates the things with the factory and then I just push messages to it. /fwiw

@jrburke
Copy link
Member

jrburke commented Aug 8, 2012

Also, I believe I can do more to allow cycles in loader plugin loaded resources, that is being tracked in general in this ticket:

requirejs/requirejs#356

but may not show up until requirejs 2.1 or later.

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

No branches or pull requests

5 participants