Element scope when $compileProvider.debugInfoEnabled(false);

[mathieumg]

Hi!

First, thanks for the nice project! It occurred to me that when setting $compileProvider.debugInfoEnabled(false); (such as when in production), this module breaks because it relies on the fact that DOM elements have a .scope property attached to them. I'm currently investigating workarounds (see angular/angular.js#9515), posting here because you may have a better idea on how to do this! :)

[fmauquie]

Hi!
Thanks :)

This one is not easy...

There are multiple ways it could be solved, and they all have issues. These are the ones I've thought about:

• Patching / PR ngReact to put the scope from their reactComponent directive as a context to React children. This not only assumes we are using ngReact (which is a safe bet), but also assumes we can get a pretty large change in their very stable codebase
• "Reverting" the debugInfoEnabled flag: This is the most versatile way, as it will always work. We could only "revert" for the scope information, by patching $compile.$$addScopeInfo and forcing it to work even when debug info is disabled. This is dirty though, as it makes the flag behave differently than specified

I've had some success with a composite approach:

1. Ask the user to 'flag' the place where they want their scope with a directive (we provide the basic linking function) to save the scope where $element.scope() knows how to find it
2. Patch ngReact if it is there to make sure all ngReact directives (and reactComponent) automatically do this
3. Don't change anything to the main code

I need to think about this a little more before releasing this, to make sure this is not too dirty and because it needs serious documentation; for anybody who would not use react-angular with ngReact, obviously, but also for the others, as the little hack requires that ngReact is loaded before react-angular.

It may also be nice to have a sensible default, such as using the root scope when the scope is not found.

I also want to somehow warn the users of the danger before they switch to production, because all these magical hacks will cause some serious headaches if anything goes wrong.

I've pushed the current state of what I have, with debug info disabled in testing and all tests green. Can you check and see if it looks OK for you ?

[Kepro]

@fmauquie can you push your changes? :) I currently have the same issue and this will help me a lot

[mathieumg]

Thanks @fmauquie for getting back to me! I currently don't use ngReact at all, if that provides a little more insight on my situation. I'm a bit overwhelmed at the moment, but I will try to take a look at your approach very soon. I tried a lot of different things to work around this issue last week, but to no avail. I thought the custom linking directive would work, but due to the linking order for nested elements, that wasn't always the case. (Elements would get their scope, but not before componentDidMount ran, and I'm not a huge fan of setting timers in these kinds of situation)

@Kepro I'm not sure which changes you are referring to exactly!

[Kepro]

sorry, updated my comment

[mathieumg]

@Kepro 10b8cce

[Kepro]

@fmauquie but can you push to npm? if I use your commit in package.json then there is missing lib folder and there is only sources...

edited: omg sorry two times wrong person :D

[mathieumg]

That is @fmauquie 's commit, and you only talked about pushing (not npm), so I was pointing you in the right direction. 😉

[fmauquie]

@Kepro I'll do this ASAP, I just need to add a little thingy to help @mathieumg :)

@mathieumg are you using ReactDOM.render() directly in a directive linking function then ? I think I can help you, I'll add a HOC to provide the scope as the component context (this may be useful to others anyways).

[mathieumg]

are you using ReactDOM.render() directly in a directive linking function then ?

Yep, it's what I'm doing! Thanks! :)

[fmauquie]

I just pushed (and released to NPM @Kepro :) ) version 0.3.0 that should fix this issue.

It ships with automatic handling for ngReact users, a HOC for manual React usage, and a directive linking function for the most extreme users.

I also updated the doc.

@Kepro , @mathieumg can you test this new version and tell me if this works for you?

[mathieumg]

@fmauquie This line errors since I don't load React through Angular:

react-angular/src/angularTemplate.js

Line 18 in 1afdb9a

const ngReactModule = angular.module('react');

You will also need to change https://github.com/fmauquie/react-angular/blob/1afdb9a612f2760b9a328cbdf3b01008293ec23b/src/index.js if we want to import different functions/classes directly from the module.

I commented out the first problem, included the file directly for the second problem, used the ensureScopeAvailable function to wrap my linker and I don't get errors, but I get the same kind of issues I did while investigating this issue last week (binding only works partially), that is (and I have no idea why) it seems either the scope that gets attached to nodes when debug is on is different or (and that is my suspicion) the order in which it gets attached then is different than the linking approach.

[fmauquie]

Right, and right... Published v0.3.1 to patch these.

[Kepro]

I still getting with 0.3.1
Uncaught TypeError: Cannot read property '$new' of undefined
at ReactAngular.componentDidMount (bundle.vendor.js:326270)

      this.$scope = scope ? parentScope.$new(isolate) : parentScope;

[fmauquie]

@Kepro I need more information in order to help you: are you using ngReact? Are you loading it before react-angular ?

If you are not using ngReact, you need to wrap your root React component in a HOC, a explained in the new section of the docs: https://github.com/fmauquie/react-angular#running-in-production

[Kepro]

yep ngReact and it's loaded in core of app

in angular template i have

      <react-component
        name="PropertiesListItem"
        props="reactProps"
        watch-depth="collection"
      />

and then in PropertiesListItem -> PropertiesEditor -> ... -> ComponentWithReact

and there is

    return (
      <AngularTemplate
        template={`<bulk-tagger definition="definition" modal-instance="modalInstance"
          signal="signal" prefix="prefix" />`}
        scope={{
          definition: this.definition,
          $close: this.close,
          modalInstance: this,
        }}
      />
    );

[fmauquie]

I think you're loading ngReact after react-angular in the loading order in your app. You can try to change this (make sure your files are loaded in the correct order, or if you're using Webpack make sure the import for ngReact appears before the one for react-angular.
If you're importing a component that imports react-angular as a dependency before importing ngReact, you will have the error too.

Do you have/make an example repo that shows the bug so I can take a look ?

[fmauquie]

I'll add a warning in development mode if I can detect an inconsistency in the module loading order (when I get time 😄)

[Kepro]

we are using webpack, I tried to add import react-angular before ngreact but same result
import 'react-angular';
import 'ngreact';

[mathieumg]

@Kepro You need to swap these two lines, the order should be the opposite.

…

On Thu, Jul 13, 2017, 6:56 AM Kepro ***@***.***> wrote: we are using webpack, I tried to add import react-angular before ngreact but same result import 'react-angular'; import 'ngreact'; — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABjK7HiGuwgW1F8VL2zkMdswmqPBnHSLks5sNiIcgaJpZM4OPnoy> .

[Kepro]

same result after i swap it

[Kepro]

why you cannot create new scope on $rootScope? because everything is there only this.context.$scope is undefined and also this.$element.scope(); is undefined... after i change this.$scope = $rootScope.new(isolate); then it start working

[fmauquie]

The idea is that you should be able to use your Angular code as if there was no React in between. In particular, if you're in a route manages by ngRouter or uiRouter with a controller defined with controllerAs, you should have access to this controller by name.

Using $rootScope as a default means that you get a different behavior in production than in dev, with the potential for very nasty and impossible to debug failures in Angular expressions.

I'm going to change the way I define the patches on ngReact so that the order of the import does not matter anymore. It will require adding an Angular module as dependency in your root module, so I'll also add a warning in development.

[fmauquie]

I released a new version that is not subject to loading order. @Kepro can you try? You need to add a module dependency to your Angular app, It is explained at the beginning of the docs.

If it does not work, I'll also add an option to use (explicitly) the root scope, and I'll be interested in a demo repo 🤔

@mathieumg this version should not change anything for you if you use the HOC. Did you get a chance to try?

[mathieumg]

@fmauquie Yeah, see my theory in the last paragraph of #1 (comment) Thanks for being super responsive in this matter, by the way! 😁 👍

[fmauquie]

@mathieumg you should try using the HOC, it will work better with a custom ReactDOM call. The scope should not be different, at least not from what I read in the AngularJS code, which does exactly what ensureScopeAvailable does 😄

No problem, I like it when people use my stuff !

[Kepro]

still... without any warning
Uncaught TypeError: Cannot read property '$new' of undefined

[fmauquie]

Hi @Kepro,

Sorry for the long time without answer, I do not have a lot of time currently.

This si very strange that you get this error. It means that current tests are inadequate (since they show no problem), could you try to provide a test in the test suite that shows it?

I'm thinking about completely removing automatic parent scope finding, and forcing users to use the HOC, as it would normalize usage and prevent any error in production (since errors would fire in development). With appropriate documentation and examples, this should be very easy to use.

Also, I could use this HOC to provide React -> Angular -> React communication (see comments on your PR).

What do you think about this approach ?