StoreDevtoolsModule.instrumentStore not compatible with AOT

[born2net]

StoreDevtoolsModule.instrumentOnlyWithExtension() works with AOT. StoreDevtoolsModule.instrumentStore not compatible with AOT getting an error of:

ERROR in Error encountered resolving symbol values statically. Calling function 'StoreDevtoolsModule', function calls are not supported. Consider replacing the function or lambda with a reference to an exported function, resolving symbol AppModule in C:/msweb/studiotouch/src/app/app.module.ts, resolving symbol AppModule in C:/msweb/studiotouch/src/app/app.module.ts

And we rely on instrumentStore as we need to provide maxAge, otherwise our dev tools will not work (large data set)

can we please fix it so it works just like instrumentOnlyWithExtension

angular-cli: 1.0.0-beta.25.5
node: 6.5.0
os: win32 x64
@angular/common: 2.4.3
@angular/compiler: 2.4.3
@angular/core: 2.4.3
@angular/forms: 2.4.3
@angular/http: 2.4.3
@angular/platform-browser: 2.4.3
@angular/platform-browser-dynamic: 2.4.3
@angular/router: 3.2.1
@angular/compiler-cli: 2.4.3
@angular/language-service: 2.4.3

thanks as always,

Sean

[juanlizarazo]

@born2net the issue is in src/instrument.ts. There is two function calls not supported by AOT (it can't analyze them statically). This file contains the method you are using.

I've fixed this and it worked (AOT succeeded) with instrumentStore() but I won't have a PR until later when I am home and can run all the tests (run out of time for now).

This is the commit

In the meantime, if this is blocking you, clone that fork, npm install, and build the release with npm run build:js && npm run build:umd && npm run build:uglify

Then the release folder is what it goes in your node_modules/@ngrx/store-devtools in your project. AOT should build fine.

If you encounter other issues, or have suggestions, post them here, as we want a good PR. I haven't contributed to this project before.

[born2net]

TX SO MUCH. No problem I will wait for the final commit, let me know when ready and I will test. TX again, Sean!!!!

[born2net]

I am ready to test whenever needed...

[juanlizarazo]

Great! I just got home. I should have said "later tonight" my bad. Looking into this right now.

[juanlizarazo]

@born2net ok! Nothing like working from your own machine. Tests are passing fine. (Better to be safe). I tested this on a project, before fix, same issue. After fix, all works and AOT completes.

You can go ahead and test in your project with the instructions I posted earlier or I can publish you a temporary build to test and use as it might take a bit (days to weeks?) for a contributor to accept my PR and version it.

[born2net]

great, so I can test now? I tried to compile and got some errors (I am on Windows 10) so if you could publish a temp npm build it would be awesome...!!!!

[juanlizarazo]

Closed PR for now. Needs work. I was troubleshooting further before giving you a build, it works with ngc (in one project I use rollup and ngc, this is the same bundler store dev tools uses to create the build). But with angular-cli (in another project), it doesn't work. I was testing this before giving you the build, by running the ng serve --aot command on that second project.

After troubleshooting further, I'm suspecting the issue for this is in this line as you can see it is the main difference between instrumentStore() and the instrumentOnlyWithExtension method.

This one uses useValue, I've run into that in the past and using a factory (as you can see in all the other providers), fixes it with AOT when exporting that factory outside the class. But, as I was attempting to do this, I run into the issue that this provided value depends on options when instrumenting the store InstrumentStore(options). [

This usually is not a problem:

function myFactory(options) {
  return function() {
    return options;
  }
}

but that requires useFactory: myFactory(options) in the same line instead of useValue, which is also a method call that can't be statically analyzed/resolved, causing the same issue.

If anyone has any ideas, or leads, that would be awesome. Off to bed now :dizzy_face:

[deebloo]

A potential fix (but would be a breaking change) would be for the user to pass a factory into instrumentStore instead of a plain object.

import {StoreDevtoolsModule} from '@ngrx/store-devtools';

export function createInstrumentOptions() {
    return {
      maxAge: 5,
      monitor: monitorReducer
    }
}
@NgModule({
  imports: [
    StoreModule.provideStore(rootReducer),
    // Note that you must instrument after importing StoreModule
    StoreDevtoolsModule.instrumentStore(createInstrumentOptions)
  ]
})
export class AppModule { }

[born2net]

+1

[born2net]

or maybe create a new function if you don't want a breaking change. something like: StoreDevtoolsModuleWithFactory or maybe test for a factory in instrumentStore

[deebloo]

could also modify the existing function to accept either a function OR the plain object since that still works with JIT. Ill fork and see how it looks

[deebloo]

@juanlizarazo you could use DI to pass the options into the fatory

[brandonroberts]

A workaround is to use the STORE_DEVTOOLS_CONFIG token

import { STORE_DEVTOOLS_CONFIG } from '@ngrx/store-devtools/src/config';

export function noMonitor() {
  return null;
}

@NgModule({
  declarations: [
    AppComponent
  ],
  imports: [
    BrowserModule,
    FormsModule,
    HttpModule,
    StoreModule.provideStore(rootReducer),
    StoreDevtoolsModule.instrumentOnlyWithExtension()
  ],
  providers: [
    {
      provide: STORE_DEVTOOLS_CONFIG, useValue: {
        maxAge: 20,
        monitor: noMonitor
      }
    }
  ],
  bootstrap: [AppComponent]
})
export class AppModule { }

[deebloo]

if there is a solution ill close my PR

[deebloo]

Here is a link to the closed PR if interested in my thought. https://github.com/ngrx/store-devtools/pull/50

[brandonroberts]

@deebloo I think a solution is needed for those who don't want to use the plugin, but it needs to be backwards compatible with using an object/function similar to what @ngrx/store does.

[born2net]

yes that works, we can close the PR!!!! TX Guys!

[born2net]

if you want me to keep the bug open let me know

[brandonroberts]

You can close it. Those who search will still be able to reference it

[born2net]

ok

[born2net]

I will open a bug to add to docs...

[juanlizarazo]

Sweet!

you could use DI to pass the options into the factory

@deebloo I tried, similar to what you were doing in your PR but most tests were breaking as they expect the exception to be thrown even before the provider object is returned. Of course I didn't succeed.

@brandonroberts Thank you for providing us with a solution!

Definitively adding a new method would be best or introducing a breaking change, as you guys suggest. I was working towards a patch to get it to work with neither api breaking changes nor new features through exposed "public" methods and aiming to keep the existing test contracts.

I will open a bug to add to docs...

@born2net Even better! you can submit a PR updating the docs.

[SamVerschueren]

Wondering why this issue was closed? It doesn't provide a solution when someone wants to use store-log-monitor. This only works with the chrome extension, right? Although the suggested solution works like a charm (thanks @brandonroberts!), it's probably not a solution if you want to debug things cross-browser. And I must say, I like the Angular 2 implementation of the monitor more then the chrome extension, but that might just be me :).

[born2net]

I can re-open if needed, let me know guys...

[brandonroberts]

I'll keep it open until a solution for both options is supported with the latest iteration of AOT