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: NGCC incorrectly compiles the "@ngxs/router-plugin" as UMD #34191

Open
arturovt opened this issue Dec 2, 2019 · 16 comments 路 May be fixed by #34254
Open

Ivy: NGCC incorrectly compiles the "@ngxs/router-plugin" as UMD #34191

arturovt opened this issue Dec 2, 2019 · 16 comments 路 May be fixed by #34254

Comments

@arturovt
Copy link
Contributor

@arturovt arturovt commented Dec 2, 2019

馃悶 bug report

Affected Package

@angular/compiler-cli

Is this a regression?

Yes

Description

The NGCC incorrectly compiles the @ngxs/router-plugin as UMD.

馃敩 Minimal Reproduction

https://github.com/arturovt/ivy-ngxs-router-plugin-umd-repro

  • clone and install dependencies
  • check the postinstall script

Or open the node_modules/@ngxs/router-plugin/bundles/ngxs-router-plugin.umd file in your IDE and see the generated code at the end of the factory(....) function. This has to be something like global...modules.ngxsFeature.module.

Providing screenshot below (this is how it looks like on my PC):

image

馃實 Your Environment

Angular Version:


Angular CLI: 9.0.0-rc.4
Node: 12.11.1
OS: linux x64

Angular: 9.0.0-rc.4
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.0-rc.4
@angular-devkit/build-angular     0.900.0-rc.4
@angular-devkit/build-optimizer   0.900.0-rc.4
@angular-devkit/build-webpack     0.900.0-rc.4
@angular-devkit/core              9.0.0-rc.4
@angular-devkit/schematics        9.0.0-rc.4
@ngtools/webpack                  9.0.0-rc.4
@schematics/angular               9.0.0-rc.4
@schematics/update                0.900.0-rc.4
rxjs                              6.5.3
typescript                        3.6.4
webpack                           4.41.2

UPD

I could guess that it emits such code due to the inability to compile this package at all. The compiler behaves differently, because:

  • in 80% of cases it compiles without errors and generates such code
  • in 20% of cases it throws during compilation with:
Error: Error on worker #1: Error: Failed to compile entry-point @ngxs/router-plugin due to compilation errors:
node_modules/@ngxs/router-plugin/bundles/ngxs-router-plugin.umd.js(1034,34): error TS-991010: Value at position 0 in the NgModule.imports of NgxsRouterPluginModule is not a reference: [object Object]
@arturovt

This comment has been minimized.

Copy link
Contributor Author

@arturovt arturovt commented Dec 2, 2019

@JoostK cc.

@arturovt

This comment has been minimized.

Copy link
Contributor Author

@arturovt arturovt commented Dec 4, 2019

I've debugged a little bit, and it seems like it resolves invalid directImport:

function getContainingImportDeclaration(node: ts.Node): ts.ImportDeclaration|null {
return ts.isImportSpecifier(node) ? node.parent !.parent !.parent ! :
ts.isNamespaceImport(node) ? node.parent.parent : null;
}

It returns a NodeObject which has a moduleSpecifier property. In my case the moduleSpecifier.text equals './modules/ngxs-feature.module'.

If I manually change it to something like (just for debugging purposes):

if (importDecl.moduleSpecifier.text === './modules/ngxs-feature.module') {
  importDecl.moduleSpecifier.text = '@ngxs/store'
}

Then it works like a charm and the UMD file is emitted w/o errors.

@markwhitfeld

This comment has been minimized.

Copy link

@markwhitfeld markwhitfeld commented Dec 5, 2019

@mgechev do you know who would be the best to look into this from the Angular Ivy team side?

@markwhitfeld

This comment has been minimized.

Copy link

@markwhitfeld markwhitfeld commented Dec 5, 2019

This is our last hurdle to releasing a fully Ivy compatible version of NGXS. 馃槂 馃殌

@petebacondarwin petebacondarwin self-assigned this Dec 9, 2019
@splincode

This comment has been minimized.

Copy link
Contributor

@splincode splincode commented Dec 9, 2019

@petebacondarwin Could you see how it turns out that?

https://circleci.com/gh/ngxs/store/23968?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Compiling @angular/platform-browser/animations : fesm2015 as esm2015
Error: Error on worker #8: Error: Failed to compile entry-point @angular/platform-browser due to compilation errors:
node_modules/@angular/core/core.d.ts(259,22): error TS-996003: Appears in the NgModule.exports of BrowserModule, but could not be resolved to an NgModule, Component, Directive, or Pipe class

    at ClusterWorker.compile (/home/circleci/workspace/app/integrations/ivy/node_modules/@angular/compiler-cli/ngcc/src/main.js:170:27)
    at Worker.<anonymous> (/home/circleci/workspace/app/integrations/ivy/node_modules/@angular/compiler-cli/ngcc/src/execution/cluster/worker.js:41:42)

image

@petebacondarwin

This comment has been minimized.

Copy link
Member

@petebacondarwin petebacondarwin commented Dec 9, 2019

I'll take a look today

@petebacondarwin

This comment has been minimized.

Copy link
Member

@petebacondarwin petebacondarwin commented Dec 10, 2019

Quick update... I am a bit lost on what is happening here.
What I can say is that if you set --async false on the ngcc invocation it does not appear to fail - at least in the numerous times I have tried it.

@gkalpak

This comment has been minimized.

Copy link
Member

@gkalpak gkalpak commented Dec 10, 2019

Smells like a dependency detection failure. I've had such issues with CommonJS (will be fixed in #34169 for most common usecases), but the same is true for UMD (iirc).

@arturovt

This comment has been minimized.

Copy link
Contributor Author

@arturovt arturovt commented Dec 10, 2019

@petebacondarwin

Hey. Thanks for the response. I've tried your approach with --async false, but it didn't help. See the screenshot below:

image

Note: those 3 dots at global...modules.ngxsFeature.module, they are added during compilation.

See this comment also #34191 (comment)

@markwhitfeld

This comment has been minimized.

Copy link

@markwhitfeld markwhitfeld commented Dec 11, 2019

Thank you so much for looking into this! Just to make sure that we are debugging the same issue please look at the issue description and @arturovt 's comments. I'm not sure if the issue that @splincode mentioned is related.
Is there any way to test if this is resolved with the PR that @gkalpak mentioned?

@gkalpak

This comment has been minimized.

Copy link
Member

@gkalpak gkalpak commented Dec 11, 2019

Sorry, I now realize I was had the error mentioned in #34191 (comment) in mind (which you said is likely unrelated). So, my PR won't help with your original issue 馃槄

@petebacondarwin

This comment has been minimized.

Copy link
Member

@petebacondarwin petebacondarwin commented Dec 11, 2019

@arturovt - sorry I was testing with some local changes (e..g #34254) and a small custom fix. With those in-place I could not get it to fail in non-async mode.
Still working on it...

@petebacondarwin

This comment has been minimized.

Copy link
Member

@petebacondarwin petebacondarwin commented Dec 11, 2019

I updated the https://github.com/arturovt/ivy-ngxs-router-plugin-umd-repro repository to point at the latest dev versions of @ngxs/router-plugin and @ngxs/store, which is 3.5.1-dev.master-7a75f33 and with the latest Angular next release (9.0.0-rc.4) I no longer get the error above (i.e. the global...modules.ngxsFeature.module).

Instead we are back to our old friend:

Compiling @ngxs/router-plugin : main as umd
Error: Error on worker #3: Error: Failed to compile entry-point @ngxs/router-plugin due to compilation errors:
node_modules/@ngxs/router-plugin/bundles/ngxs-router-plugin.umd.js(1034,34): error TS-991010: Value at position 0 in the NgModule.imports of NgxsRouterPluginModule is not a reference: [object Object]
@petebacondarwin

This comment has been minimized.

Copy link
Member

@petebacondarwin petebacondarwin commented Dec 11, 2019

Updating Angular to use the artifacts from #34254 removes this error and the ngcc processing seems to complete without error.

That being said I now see that it adds an incorrect export into the node_modules/@ngxs/store/ngxs-store.d.ts file:

export {傻l as NgxsRootModule} from './src/modules/ngxs-root.module';
export {傻x as NgxsFeatureModule} from './src/modules/ngxs-feature.module';

It is confusing the public export from this file:

export { NgxsFeatureModule as 傻x } from './src/modules/ngxs-feature.module';
export { NgxsRootModule as 傻l } from './src/modules/ngxs-root.module';

with the exports from the referenced files.

@petebacondarwin

This comment has been minimized.

Copy link
Member

@petebacondarwin petebacondarwin commented Dec 11, 2019

Actually it additionally needs this PR to complete successfully: #34356

@petebacondarwin

This comment has been minimized.

Copy link
Member

@petebacondarwin petebacondarwin commented Dec 12, 2019

OK so now that #34356 has landed, I believe that #34254 should be enough to fix this package, assuming that you are using the latest dev version of the package. Hopefully it will be merged this week and appear in the next RC next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can鈥檛 perform that action at this time.