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

Not working due to the isDependecyUsedInAnyDeclaration check #26

Closed
buunguyen opened this issue Mar 27, 2015 · 12 comments
Closed

Not working due to the isDependecyUsedInAnyDeclaration check #26

buunguyen opened this issue Mar 27, 2015 · 12 comments

Comments

@buunguyen
Copy link

I have an app where a module is contained in multiple files in a folder. There's a main file which declares the module (i.e. angular.module('mod', [])) and other files that add stuff to it (e.g. angular.module('mod').controller('c1')). gulp-angular-filesort doesn't sort files correctly at all. However, if if I comment out the check isDependecyUsedInAnyDeclaration, it starts working.

I'm not exactly sure what this check is for and why it is needed. Is it an optimization? If it is, it breaks the case like my app where a module is contained in multiple files. Can you shed some light? Thank you.

@buunguyen buunguyen changed the title Not working due to the isDependecyUsedInAnyDeclaration check Not working due to the isDependecyUsedInAnyDeclaration check Mar 27, 2015
@joakimbeng
Copy link
Member

Does the sort order break your whole app? I.e. does Angular give you module loading errors?
The scenario you describes is working for me with this module, so I need more information of what exactly is not working, and preferably with an example of your directory/file structure.

@buunguyen
Copy link
Author

@joakimbeng thanks for responding. Yes, the sort order breaks the app and I also get the module loading error. My observation is that when the check exists, many files in the project aren't added to the toSort list, resulting in them not being sorted.

I will try to reproduce the issue in a clean project and share more information when I can.

@buunguyen
Copy link
Author

You can see the issue in this repo. Just run npm install && gulp to generate index.html. There are 3 modules, users, core and app. app depends on users which depends on core. But the injected script order is: user -> core -> app instead of core -> user -> app.

@buunguyen
Copy link
Author

@joakimbeng, just want to check in if there's any update. Thanks!

@simon04
Copy link

simon04 commented Aug 7, 2015

I assume that there's a misunderstanding on the purpose of the sorting: this gunt module sorts modules according "define before use", but does not sort module dependencies in front of modules using those: See #7 (comment)

Maybe that should be clarified in the README.

@buunguyen
Copy link
Author

@simon04 I would argue that "use" = "depend" (i.e. A uses B is similar to A depends on B). So the plugin should support dependencies as well. Otherwise, it wouldn't work even for a simple (yet realistic) example in the repo I provided.

1 similar comment
@buunguyen
Copy link
Author

@simon04 I would argue that "use" = "depend" (i.e. A uses B is similar to A depends on B). So the plugin should support dependencies as well. Otherwise, it wouldn't work even for a simple (yet realistic) example in the repo I provided.

@simon04
Copy link

simon04 commented Aug 7, 2015

Issue #25 is related, see especially #25 (comment).

  • define: angular.module('module', []);
  • define with dependencies: angular.module('module', ['other_module']);
  • use: angular.module('module')...

"Define" has to come before "use". But it does not matter whether the other_module definition comes before the module definition since angular resolves this.

@buunguyen
Copy link
Author

I hear you. I (and probably those submitting similar tickets) need correct order for dependencies because I define stuff in module A (e.g. BaseController) that is used in module B depending on A.

Per my question, the order works correctly if the check isDependecyUsedInAnyDeclaration is commented out. So the question is why having it in the first place? Is it an optimization?

@simon04
Copy link

simon04 commented Aug 7, 2015

isDependecyUsedInAnyDeclaration has been added in the course of #7.

"I need correct order for dependencies": angular does not, and this gulp task does not provide it.

I created a small gist to show that a module A depending on B can be loaded before B (as your example where A and B are swapped): https://gist.github.com/simon04/031d602c550d31ea3778

@buunguyen
Copy link
Author

@simon04 thanks for providing the gist.

If you add a global var like BaseController to B-define.js and use it from A-define.js, it wouldn't work because B-define.js comes before A-define.js. And that's the use case I'm after.

Regarding the linked issue #7, I suppose there's a way to fix the cyclic dep error while still ensuring dependency order. But I might be wrong.

@joakimbeng
Copy link
Member

@buunguyen why do you use global variables? All best practices discourages that, and so does this plugin.

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

3 participants