Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ivy fails with NGXS #31791

Closed
arturovt opened this issue Jul 23, 2019 · 16 comments

Comments

@arturovt
Copy link

commented Jul 23, 2019

馃悶 bug report

Is this a regression?

Yes, it is

Description

NGXS works with Angular 8, but build fails when enableIvy is true

馃敩 Minimal Reproduction

https://github.com/arturovt/ivy-ngxs-repro

  • create new app with --enable-ivy
  • install @ngxs/store
  • import the NgxsModule
  • build with --prod flag

馃敟 Exception or Error


chunk {0} runtime-es5.465c2333d355155ec5f3.js (runtime) 1.41 kB [entry] [rendered]
chunk {1} main-es5.f306d8f70fdeea68d975.js (main) 178 kB [initial] [rendered]
chunk {2} polyfills-es5.e0a0858fa7791e140ae9.js (polyfills) 113 kB [initial] [rendered]
Date: 2019-07-23T07:08:02.084Z - Hash: 93ed973fa7b88e080f3b - Time: 17265ms

ERROR in ./src/app/app.module.ts 19:104-121
"export 'NgxsRootModule' (imported as 'i1') was not found in '@ngxs/store'

馃實 Your Environment

Angular Version:


Angular CLI: 9.0.0-next.2
Node: 12.6.0
OS: linux x64
Angular: 9.0.0-next.5
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Anything else relevant?
May be related to the #29939

UPD

It's built without errors using CLI 9.0.0-next.2 version. But there is a runtime error after importing NgxsRouterPluginModule:

Can't resolve all parameters for RouterState: (?, ?, ?, ?, ?, ?, ?).

Because there is no ngInjectableDef property on the RouterState.

Also when running ivy-ngcc it fails compiling @ngxs/router-plugin as umd:

Value at position 0 in the NgModule.imports of NgxsRouterPluginModule is not a reference: [object Object]

@markwhitfeld

This comment has been minimized.

Copy link

commented Jul 23, 2019

Just to add in here, I have just tested this repo with Angular 8.2.0-next.2 and it has the same build failures.
In the referenced firebase issue it was suggested that this would be fixed by PR #30591 but this PR seems to have been merged, closed (although there seem to be more subsequent commits on it?) and is part of the Angular 8.2.0-next.2 package. The issue is unfortunately still present in this version.

@splincode

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2019

@alxhub @AndrewKushnir Angular 9.beta + NGXS works now, but @ngxs/router-plugin not working with Ivy

for reproduction

app.module.ts

import { BrowserModule } from '@angular/platform-browser';
import { RouterModule } from '@angular/router';
import { NgModule } from '@angular/core';

import { AppComponent } from './app.component';
import { StoreIvyModule } from './store-ivy.module';

@NgModule({
  declarations: [AppComponent],
  imports: [
    BrowserModule,
    StoreIvyModule,
    RouterModule.forRoot([{ path: '', component: AppComponent }])
  ],
  bootstrap: [AppComponent]
})
export class AppModule {}

app.state.ts

import { State } from '@ngxs/store';

@State<string[]>({
  name: 'app',
  defaults: []
})
export class AppState {}

store-ivy.module.ts

import { NgModule } from '@angular/core';
import { NgxsModule } from '@ngxs/store';
import { NgxsReduxDevtoolsPluginModule } from '@ngxs/devtools-plugin';
import { NgxsFormPluginModule } from '@ngxs/form-plugin';
import { NgxsLoggerPluginModule } from '@ngxs/logger-plugin';
import { NgxsStoragePluginModule } from '@ngxs/storage-plugin';
import { NgxsWebsocketPluginModule } from '@ngxs/websocket-plugin';
import { NgxsRouterPluginModule } from '@ngxs/router-plugin';

import { AppState } from './app.state';

@NgModule({
  imports: [
    NgxsModule.forRoot([AppState]),
    NgxsReduxDevtoolsPluginModule.forRoot(),
    NgxsFormPluginModule.forRoot(),
    NgxsLoggerPluginModule.forRoot(),
    NgxsStoragePluginModule.forRoot(),
    NgxsWebsocketPluginModule.forRoot(),
    NgxsRouterPluginModule.forRoot()
  ],
  exports: [NgxsModule]
})
export class StoreIvyModule {}

Build successful, but runtime error after ng serve

Can't resolve all parameters for RouterState: (?, ?, ?, ?, ?, ?, ?).

But works with Angular 7, 8 with ViewEngine

@somoy99

This comment has been minimized.

Copy link

commented Sep 4, 2019

I have the same problem with --enable-ivy on, NgxsRouterPluginModule does not work, gives me the same error, rests are okay

@arturovt

This comment has been minimized.

Copy link
Author

commented Sep 5, 2019

@alxhub @AndrewKushnir

FYI I've updated Angular version to the 9.0.0-next.5 in this repro.

It's built without errors but still there is a runtime error Can't resolve all parameters for RouterState: (?, ?, ?, ?, ?, ?, ?)..

When running ivy-ngcc, it fails when Compiling @ngxs/router-plugin : main as umd with:

Value at position 0 in the NgModule.imports of NgxsRouterPluginModule is not a reference: [object Object]

Also I've noticed that it fails with Can't resolve all parameters for RouterState because there is no ngInjectableDef and if I add it manually, then in works:

RouterState.ngInjectableDef = defineInjectable({
  factory: () =>
    new RouterState(
      inject(Store),
      inject(Router),
      inject(<any>RouterStateSerializer),
      inject(NgZone),
      inject(<any>UrlSerializer),
      inject(<any>LocationStrategy),
      inject(Location)
    ),
  token: RouterState,
  providedIn: 'root'
});
@AndrewKushnir

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2019

Hi @arturovt,

Thanks for additional information. I've looked at the code and it seems like the issue appears because there is insufficient type information provided when you use ModuleWithProviders type, for example here, it should be ModuleWithProviders<NgxsModule>, see docs for ModuleWithProviders type here.

Please try to update all usages of the ModuleWithProviders type in your project, try to compile it with Ivy again and let us know if that resolved the problem. Note: this change is also compatible with View Engine.

Thank you.

@crywolfe

This comment has been minimized.

Copy link

commented Sep 10, 2019

I'm having the same issue with a project I'm working on. @AndrewKushnir, the NgxsRouterModule plugin already has the type, specifically <NgxsRouterPluginModule>. Isn't this an issue for the Ngxs team to fix?

    static forRoot(): ModuleWithProviders<NgxsRouterPluginModule>;
    static ngModuleDef: 傻ngcc0.傻傻NgModuleDefWithMeta<NgxsRouterPluginModule, never, [typeof 傻ngcc1.NgxsFeatureModule], never>;
    static ngInjectorDef: 傻ngcc0.傻傻InjectorDef<NgxsRouterPluginModule>;
}
@arturovt

This comment has been minimized.

Copy link
Author

commented Sep 10, 2019

Hey @AndrewKushnir,

I tried your approach with adding module type to all ModuleWithProviders. Unfortunately it didn't help. I went a little bit deeper and found NgModuleDecoratorHandler.resolveTypeList that throws this exception.

I logged an entry variable that is instanceof DynamicValue. I saw that DynamicValue references ts.Node so I invoked entry.node.getFullText(). Your assumption is confirmed, it cannot determine type definition when compiling umd.

It prints export declare class NgxsFeatureModule { ... } when compiling fesm5/2015, esm5/2015. But prints store.NgxsModule.forFeature([RouterState]) when tries to compile in the umd format.

Are there additional workarounds I could try?

@JoostK JoostK self-assigned this Sep 11, 2019
JoostK added a commit to JoostK/angular that referenced this issue Sep 11, 2019
In ESM2015 bundles, a class with decorators may be emitted as follows:

```javascript
var MyClass_1;
let MyClass = MyClass_1 = class MyClass {};
MyClass.decorators = [/* here be decorators */];
```

Such a class has two declarations: the publicly visible `let MyClass`
and the implementation `class MyClass {}` node. In angular#32539 a refactoring
took place to handle such classes more consistently, however the logic
to find static properties was mistakenly kept identical to its broken
state before the refactor, by looking for static properties on the
implementation symbol (the one for `class MyClass {}`) whereas the
static properties need to be obtained from the symbol corresponding with
the `let MyClass` declaration, as that is where the `decorators`
property is assigned to in the example above.

This commit fixes the behavior by looking for static properties on the
public declaration symbol. This fixes an issue where decorators were not
found for classes that do in fact have decorators, therefore preventing
the classes from being compiled for Ivy.

Fixes angular#31791
@JoostK

This comment has been minimized.

Copy link
Member

commented Sep 11, 2019

@arturovt Andrew asked me to look into it. I was able to reproduce the issue you described for the Can't resolve all parameters for RouterState: (?, ?, ?, ?, ?, ?, ?). error at runtime, for which I opened #32619 with a fix.

I don't know about the UMD issues you're experiencing, but it is likely to fail due to a different root cause. What can I do to reproduce the UMD failure? Thanks!

@arturovt

This comment has been minimized.

Copy link
Author

commented Sep 11, 2019

@JoostK could you try to run yarn ivy-ngcc -p main in the project that has @ngxs/store and @ngxs/router-plugin as dependencies?

JoostK added a commit to JoostK/angular that referenced this issue Sep 11, 2019
In ESM2015 bundles, a class with decorators may be emitted as follows:

```javascript
var MyClass_1;
let MyClass = MyClass_1 = class MyClass {};
MyClass.decorators = [/* here be decorators */];
```

Such a class has two declarations: the publicly visible `let MyClass`
and the implementation `class MyClass {}` node. In angular#32539 a refactoring
took place to handle such classes more consistently, however the logic
to find static properties was mistakenly kept identical to its broken
state before the refactor, by looking for static properties on the
implementation symbol (the one for `class MyClass {}`) whereas the
static properties need to be obtained from the symbol corresponding with
the `let MyClass` declaration, as that is where the `decorators`
property is assigned to in the example above.

This commit fixes the behavior by looking for static properties on the
public declaration symbol. This fixes an issue where decorators were not
found for classes that do in fact have decorators, therefore preventing
the classes from being compiled for Ivy.

Fixes angular#31791
@JoostK

This comment has been minimized.

Copy link
Member

commented Sep 11, 2019

@arturovt thanks, that does indeed reproduce the issue you describe for store.NgxsModule.forFeature([RouterState]). I marked #32619 as WIP for now and look into the UMD issue tomorrow, so that we can hopefully resolve both issues with that PR.

JoostK added a commit to JoostK/angular that referenced this issue Sep 12, 2019
In ESM2015 bundles, a class with decorators may be emitted as follows:

```javascript
var MyClass_1;
let MyClass = MyClass_1 = class MyClass {};
MyClass.decorators = [/* here be decorators */];
```

Such a class has two declarations: the publicly visible `let MyClass`
and the implementation `class MyClass {}` node. In angular#32539 a refactoring
took place to handle such classes more consistently, however the logic
to find static properties was mistakenly kept identical to its broken
state before the refactor, by looking for static properties on the
implementation symbol (the one for `class MyClass {}`) whereas the
static properties need to be obtained from the symbol corresponding with
the `let MyClass` declaration, as that is where the `decorators`
property is assigned to in the example above.

This commit fixes the behavior by looking for static properties on the
public declaration symbol. This fixes an issue where decorators were not
found for classes that do in fact have decorators, therefore preventing
the classes from being compiled for Ivy.

Fixes angular#31791
JoostK added a commit to JoostK/angular that referenced this issue Sep 12, 2019
In ngcc's reflection host for UMD and CommonJS bundles, custom logic is
present to resolve import details of an identifier. However, this custom
logic is unable to resolve an import for an identifier inside of
declaration files, as such files use the regular ESM import syntax.

As a consequence of this limitation, ngtsc is unable to resolve
`ModuleWithProviders` imports that are declared in an external library.
In that situation, ngtsc determines the type of the actual `NgModule`
that is imported, by looking in the library's declaration files for the
generic type argument on `ModuleWithProviders`. In this process, ngtsc
resolves the import for the `ModuleWithProviders` identifier to verify
that it is indeed the `ModuleWithProviders` type from `@angular/core`.
So, when the UMD reflection host was in use this resolution would fail,
therefore no `NgModule` type could be detected.

This commit fixes the bug by using the regular import resolution logic
in addition to the custom resolution logic that is required for UMD
and CommonJS bundles.

Fixes angular#31791
@JoostK

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

@arturovt there was indeed an issue with resolving store.NgxsModule.forFeature([RouterState]) in both CommonJS and UMD bundles, for which I included a fix in #32619. With those fixes, the compilation no longer fails :-)

@arturovt

This comment has been minimized.

Copy link
Author

commented Sep 12, 2019

@JoostK great, dank je wel :)

@kara kara closed this in c4e039a Sep 12, 2019
kara added a commit that referenced this issue Sep 12, 2019
鈥32619)

In ngcc's reflection host for UMD and CommonJS bundles, custom logic is
present to resolve import details of an identifier. However, this custom
logic is unable to resolve an import for an identifier inside of
declaration files, as such files use the regular ESM import syntax.

As a consequence of this limitation, ngtsc is unable to resolve
`ModuleWithProviders` imports that are declared in an external library.
In that situation, ngtsc determines the type of the actual `NgModule`
that is imported, by looking in the library's declaration files for the
generic type argument on `ModuleWithProviders`. In this process, ngtsc
resolves the import for the `ModuleWithProviders` identifier to verify
that it is indeed the `ModuleWithProviders` type from `@angular/core`.
So, when the UMD reflection host was in use this resolution would fail,
therefore no `NgModule` type could be detected.

This commit fixes the bug by using the regular import resolution logic
in addition to the custom resolution logic that is required for UMD
and CommonJS bundles.

Fixes #31791

PR Close #32619
@crywolfe

This comment has been minimized.

Copy link

commented Sep 13, 2019

Hi @JoostK. Still getting this error after updating package.json with angular 8.2.6. Any suggestions?

Compiling @ngxs/router-plugin : main as umd
Value at position 0 in the NgModule.imports of NgxsRouterPluginModule is not a reference: [object Object]
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! nam-client@0.0.0 postinstall: `ivy-ngcc`
@arturovt

This comment has been minimized.

Copy link
Author

commented Sep 13, 2019

@crywolfe check their changelog, they haven't published anything for the last 2 days ;)

@jtomek

This comment has been minimized.

Copy link

commented Sep 16, 2019

FYI, NGXS team has not checked the fix yet because they're waiting for the a new version of the compiler to be published. Ref: ngxs/store#1165

arnehoek added a commit to arnehoek/angular that referenced this issue Sep 26, 2019
鈥ar#32619)

In ESM2015 bundles, a class with decorators may be emitted as follows:

```javascript
var MyClass_1;
let MyClass = MyClass_1 = class MyClass {};
MyClass.decorators = [/* here be decorators */];
```

Such a class has two declarations: the publicly visible `let MyClass`
and the implementation `class MyClass {}` node. In angular#32539 a refactoring
took place to handle such classes more consistently, however the logic
to find static properties was mistakenly kept identical to its broken
state before the refactor, by looking for static properties on the
implementation symbol (the one for `class MyClass {}`) whereas the
static properties need to be obtained from the symbol corresponding with
the `let MyClass` declaration, as that is where the `decorators`
property is assigned to in the example above.

This commit fixes the behavior by looking for static properties on the
public declaration symbol. This fixes an issue where decorators were not
found for classes that do in fact have decorators, therefore preventing
the classes from being compiled for Ivy.

Fixes angular#31791

PR Close angular#32619
arnehoek added a commit to arnehoek/angular that referenced this issue Sep 26, 2019
鈥gular#32619)

In ngcc's reflection host for UMD and CommonJS bundles, custom logic is
present to resolve import details of an identifier. However, this custom
logic is unable to resolve an import for an identifier inside of
declaration files, as such files use the regular ESM import syntax.

As a consequence of this limitation, ngtsc is unable to resolve
`ModuleWithProviders` imports that are declared in an external library.
In that situation, ngtsc determines the type of the actual `NgModule`
that is imported, by looking in the library's declaration files for the
generic type argument on `ModuleWithProviders`. In this process, ngtsc
resolves the import for the `ModuleWithProviders` identifier to verify
that it is indeed the `ModuleWithProviders` type from `@angular/core`.
So, when the UMD reflection host was in use this resolution would fail,
therefore no `NgModule` type could be detected.

This commit fixes the bug by using the regular import resolution logic
in addition to the custom resolution logic that is required for UMD
and CommonJS bundles.

Fixes angular#31791

PR Close angular#32619
@angular-automatic-lock-bot

This comment has been minimized.

Copy link

commented Oct 17, 2019

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can鈥檛 perform that action at this time.