Skip to content
This repository was archived by the owner on Jun 7, 2021. It is now read-only.

Using inject attribute instead of $inject; #100

Closed
wants to merge 1 commit into from

Conversation

alber70g
Copy link

The Inject() could also be put in /src/app/utils/injectAttribute for example. But this makes it easier to reference. Just use a number of ../

@dsebastien
Copy link
Owner

Hi @alber70g,

Thanks for submitting this PR!
I'm not too fond of the approach you propose because I have some other ideas; I'm open for discussion though!

Here are some ideas I have around handling dependency injection, imports, etc. In two words, I'd like to get rid of most ../../.. style imports/references as that's just not maintainable and cumbersome to write.

  1. add ng-annotate & configure webpack plugin (Add ng-annotate #103)

I think this one would be a cleaner solution to this particular issue (injecting AngularJS deps without breaking on minification)

  1. add aliases to Webpack config (Add aliases #102)

Through that, we could put reusable aliases in one central location, but this might not be the cleanest approach..

  1. add a global requireRoot utility method (Add global requireRoot hook #76)

This one could be nice actually because we could simply define "src" as the root and whenever something must be required from some location under src, we can just give a relative path from there. The only case where this isn't great is when there's a lot of nesting (which should be avoided anyways).

  1. add ng-forward (Add ng-forward #101)

This one is very appealing to me although I'm not sure the project is still maintained...

@alber70g
Copy link
Author

Thank you for this thorough response 😄
For me the ../../ is indeed very cumbersome, in general the way of dependencies and references is not too clean. Indeed the requireRoot would follow a more 'namespace'/'package' approach.

I'll try a few out in my current projects and see if there are pro's/con's

@dsebastien
Copy link
Owner

Closing for now; don't hesitate to reopen later or to submit a PR :)

@dsebastien dsebastien closed this May 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants