Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

compatibility with opal.js #2653

Closed
schmurfy opened this issue May 14, 2013 · 8 comments
Closed

compatibility with opal.js #2653

schmurfy opened this issue May 14, 2013 · 8 comments

Comments

@schmurfy
Copy link

Hi,

While trying to load both opal and angularjs I got an error in angular.js with a really "helpful" message in the chrome console:
"Uncaught TypeError: object is not a function from ng"

Here is my minimal test page page:

<html ng-app>
  <head>
    <script type="text/javascript" src="js/angular.js"></script>
    <script type="text/javascript" src="js/opal.js"></script>
  </head>
  <body>
  </body>
</html>

opal alone works fine, angularjs alone works fine but with both loaded, boom !
If someone has any idea on what might be happening I am pretty lost as to what I could try.

The opal.js file can be found here: http://cdnjs.cloudflare.com/ajax/libs/opal/0.3.43/opal.js, I have tested this with angular 1.1.4.
@lgalfaso
Copy link
Contributor

The code below (and the like) from opal.js cannot be good for other libraries

  Object.prototype.toString = function() {
    return this.$to_s();
  };

@schmurfy schmurfy mentioned this issue May 15, 2013
Closed
@lgalfaso
Copy link
Contributor

lgalfaso commented Jul 8, 2013

I've looked at this a little bit more, and it looks like all the incompatibilities can be reduced to two cases

  • opal adds a property $inject to arrays
  • opal overrides Array.prototype.toString

The first of these is something that it is possible to work around, the only change needed is at the function createInjector subfunction createInternalInjector subfunction invoke change

      if (!fn.$inject) {
        // this means that we must be an array.
        fn = fn[length];
      }

for

      if (isArray(fn)) {
        fn = fn[length];
      }

with this change almost everything works.
The second issue, this is the fact that opal overrides Array.prototype.toString, affects only if Array is defined as a factory, even when this is unusual, is not possible to work around this without an ugly hack.

Note: All test were performed using jqLite, and it is expected that using jQuery there will be other incompatibilities that will need to be uncovered

I think that this issue can be closed, if there is anything else, then or opal should behave better or developers that would like to use both libraries will have to be super careful and patch their local angular.js

@schmurfy
Copy link
Author

schmurfy commented Jul 8, 2013

Thanks for digging into this :)

@peterneubauer
Copy link

Is there any way I can make this work right now or is this still usupported?

@lgalfaso
Copy link
Contributor

@peterneubauer There are two options

  • You ask the opal team not to add items to Array.prototype (or that opal uses a class that extends it and the extension does add $inject and toString)
  • You manually patch your version of angular.js and add the change at compatibility with opal.js #2653 (comment)

BTW, the first option would be the option I think works best

@peterneubauer
Copy link

Thanks @lgalfaso , will try that!

@peterneubauer peterneubauer mentioned this issue Oct 15, 2013
Closed
@elia
Copy link

elia commented Oct 18, 2013

Just want to point out that .toString() isn't an issue

I can say that we do not redefine toString for arrays. Maybe it was accidentally overridden at the time when it was looked into, but it is not affected on opal as it stands. (see opal/opal#400)

On my side I think that the isArray check is more correct anyway

@rubys
Copy link

rubys commented Oct 18, 2013

If I read correctly, the current preferred approach by the Opal developers is to release code that monkey patches Angular.js. Given this, can I inquire as to why !fn.$inject would be preferred over isArray(fn)?

@tomchentw tomchentw mentioned this issue Dec 3, 2013
Closed
MichaelSp added a commit to MichaelSp/angular.js that referenced this issue Jun 19, 2014
@MichaelSp
opal compatability as suggested in angular#2653
75ca109
@MichaelSp MichaelSp mentioned this issue Jun 19, 2014
Closed
IgorMinar added a commit that referenced this issue Jun 24, 2014
@IgorMinar
fix($injector): check if a fn is an array explicitly
67c11b9 
This change makes the code easier to read and also fixes a compatibility issue
with opal.js which pollutes the global state by setting $inject property on
Array prototype

Closes #7904
Closes #2653
IgorMinar added a commit that referenced this issue Jun 24, 2014
@IgorMinar
fix($injector): check if a fn is an array explicitly
b1a6baa 
This change makes the code easier to read and also fixes a compatibility issue
with opal.js which pollutes the global state by setting $inject property on
Array prototype

Closes #7904
Closes #2653
ckknight pushed a commit to ckknight/angular.js that referenced this issue Jul 16, 2014
@IgorMinar
fix($injector): check if a fn is an array explicitly
cf6ba31 
This change makes the code easier to read and also fixes a compatibility issue
with opal.js which pollutes the global state by setting $inject property on
Array prototype

Closes angular#7904
Closes angular#2653
@theurere theurere mentioned this issue Aug 26, 2014
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@elia @schmurfy @rubys @peterneubauer @lgalfaso @pkozlowski-opensource