Skip to content
This repository has been archived by the owner on May 25, 2019. It is now read-only.

[RFR] Fix Strict DI issues and remove ngAnnotate #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jpetitcolas
Copy link

@jpetitcolas jpetitcolas commented Sep 13, 2016

This PR aims to solve an issue when importing this module in a Strict Dependency Injection environment. Indeed, using current version, installed with npm, we get the following Angular error:

Error: [$injector:strictdi] http://errors.angularjs.org/1.4.12/$injector/strictdi?p0=uiCodemirrorDirective

Bower version is not impacted as ng-annotate plugin seems to be used in this case.

As source file is already compatible with strict DI, I also removed the obsolete ng-annotate plugin.

@jpetitcolas
Copy link
Author

Fixes #106.

@genu
Copy link

genu commented May 9, 2017

Not sure why removing ng-annotate is the solution here. I figure it should work with ng-annotate as expected in npm as long as the main file points to the dist version. #138 Attempts to make the same change.

Also, why aren't these PRs reviewed? Looks like this hasn't been touched for a year.

@jpetitcolas
Copy link
Author

@genu: the main fix is here.

Removing ng-annotate has been done because it is not required anymore, as code is compatible with strict DI. But that's a side-effect of the fix.

@postama
Copy link

postama commented Mar 19, 2018

What is the status of this PR. It has been almost a year and this fix is necessary for us. Let me know if there is anything that needs to be done to allow it to get merged in.

@jpetitcolas
Copy link
Author

Same here. We got a production project using my fork as a dependency. That's not really comfortable. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants