vendure-ecommerce / vendure

The commerce platform with customization in its DNA.
https://www.vendure.io
Other
5.67k stars 1.01k forks source link

Admin Ui: Angular error when compiling AdminUiPlugin #758

Closed Wanztwurst closed 3 years ago

Wanztwurst commented 3 years ago

Compiling the AdminUiPlugin works with devMode since 1.0.0-beta.3 again, but now the compiled admin-ui throws an error in the Angular code when opening the admin page in browser: TypeError: e.factory is not a function

To Reproduce

  1. Start vendure server with AdminUiPlugin.init({ ... app: compileUiExtensions({ ... }) )
  2. Open admin page in Browser
  3. main.ts:16 TypeError: e.factory is not a function – page doesn't load

Empty extensions: [] or parameters, including devMode, don't seem to make a difference.

michaelbromley commented 3 years ago

I ran into some issues with Angular after updating, and the solution in my case seemed to be to delete the lockfile, node_modules, and then reinstall. If you've not tried that yet, have a go and let me know if it solves it.

Wanztwurst commented 3 years ago

This didn't change the outcome. Sadly I have no idea what could cause this issue.

michaelbromley commented 3 years ago

OK. Is there more to the stack trace you can share?

Wanztwurst commented 3 years ago

Last year some people experienced the bug after an Angular upgrade from 8 to 9, there was something wrong with a decorator. But I compiled the admin-ui successfully after the upgrade to Angular 11 last week – no clue again.

Chromium:

TypeError: record.factory is not a function
    at R3Injector.hydrate (core.js:11379)
    at R3Injector.get (core.js:11200)
    at core.js:11237
    at Set.forEach (<anonymous>)
    at R3Injector._resolveInjectorDefTypes (core.js:11237)
    at new NgModuleRef$1 (core.js:25268)
    at NgModuleFactory$1.create (core.js:25322)
    at core.js:29133
    at ZoneDelegate.invoke (zone-evergreen.js:372)
    at Object.onInvoke (core.js:28534)

Firefox:

TypeError: record.factory is not a function
    Angular 20
        hydrate
        get
        _resolveInjectorDefTypes
        _resolveInjectorDefTypes
        NgModuleRef$1
        create
        bootstrapModuleFactory
        invoke
        onInvoke
        invoke
        run
        run
        bootstrapModuleFactory
        bootstrapModule
        invoke
        run
        scheduleResolveOrReject
        invokeTask
        runTask
        drainMicroTaskQueue
main.ts:16:16

This is the error trace from the browser – is there a better one I could get you? Also, does it currently work on your system – then it should be an error on my end?

michaelbromley commented 3 years ago

Thanks, the non-minified stack trace is much more useful.

I have a project with quite a lot of custom UI stuff being compiled. Didn't have any issue with the latest beta.3 (after deleting lockfile as mentioned).

So take another look at your end, let me know if you find anything else useful. If not, then a minimal reproduction repo would be helpful for me to debug.

Wanztwurst commented 3 years ago

Will do, thank you!

martijnvdbrug commented 3 years ago

I found this related issue: https://github.com/angular/angular/issues/35282, ~~where some comments say that upgrading added unwanted @Injectable()'s to components. In packages/admin-ui/src/lib/core/src/shared/pipes/locale-base.pipe.ts I see an Injectable() on abstract class LocaleBasePipe, which should not be there, because this is an abstract class.~~ Nope, this is not it.

michaelbromley commented 3 years ago

I went though https://github.com/angular/angular/issues/35282 and tried to re-create the error in the real-world-vendure repo:

Neither of those reproduced the error. So I'm a bit stuck in fixing this without a reproduction.

One question I have is this: did you delete the admin-ui folder after upgrading?

martijnvdbrug commented 3 years ago

One question I have is this: did you delete the admin-ui folder after upgrading? Yep.

I think I have a minimal reproduction. It does use createTestEnvironment instead of a 'real' vendure instance, but the error is the same:

  1. Run this with yarn ts-node dev-server.ts
  2. Go to http://localhost:3050/admin and see TypeError: record.factory is not a function in the console.
    
    // dev-server.ts
    import {createTestEnvironment, registerInitializer, SqljsInitializer, testConfig,} from '@vendure/testing';
    import {DefaultLogger, LogLevel} from '@vendure/core';
    import {AdminUiPlugin} from '@vendure/admin-ui-plugin';
    import {compileUiExtensions} from '@vendure/ui-devkit/compiler';
    import * as path from 'path';
    import {initialData} from '../../test/initialData';

(async () => { testConfig.logger = new DefaultLogger({level: LogLevel.Debug}); registerInitializer('sqljs', new SqljsInitializer('data')); testConfig.plugins.push(AdminUiPlugin.init({ route: 'admin', port: 3002, app: compileUiExtensions({ outputPath: path.join(dirname, 'admin-ui'), extensions: [], devMode: true, }), })); testConfig.apiOptions.shopApiPlayground = {}; testConfig.apiOptions.adminApiPlayground = {}; const {server} = createTestEnvironment(testConfig); await server.init({ initialData, productsCsvPath: '../test/products-import.csv', }); })();

Dependency versions:
```js
// yarn list --pattern "@angular/*"
yarn list v1.22.10
├─ @angular/animations@11.2.4
├─ @angular/cdk@11.2.3
├─ @angular/cli@11.2.4
├─ @angular/common@11.2.4
├─ @angular/compiler-cli@11.2.5
├─ @angular/compiler@11.2.5
├─ @angular/core@11.2.4
├─ @angular/forms@11.2.4
├─ @angular/language-service@11.2.4
├─ @angular/platform-browser-dynamic@11.2.4
├─ @angular/platform-browser@11.2.4
└─ @angular/router@11.2.4

ts-node

yarn list v1.22.10
└─ ts-node@9.1.1

typescript

yarn list v1.22.10
└─ typescript@4.0.5
michaelbromley commented 3 years ago

Great, thanks.

Initially I could not reproduce the error. My angular deps looked like:

$ yarn list --pattern "@angular/*"
yarn list v1.19.1
├─ @angular/animations@11.2.4
├─ @angular/cdk@11.2.3
├─ @angular/cli@11.2.3
├─ @angular/common@11.2.4
├─ @angular/compiler-cli@11.2.4
├─ @angular/compiler@11.2.4
├─ @angular/core@11.2.4
├─ @angular/forms@11.2.4
├─ @angular/language-service@11.2.4
├─ @angular/platform-browser-dynamic@11.2.4
├─ @angular/platform-browser@11.2.4
└─ @angular/router@11.2.4

Then I deleted the lockfile and reinstalled, so my Angular deps were the same as your list. Now I get the error.

The specific packages changed are:

@angular/cli@11.2.3   -> 11.2.4
@angular/compiler-cli@11.2.4   -> 11.2.5
@angular/compiler@11.2.4  -> 11.2.5

So a work-around for now would be to use yarn resolutions to pin those 3 Angular packages to the previous version.

michaelbromley commented 3 years ago

OK I think I have the reason right here: https://github.com/angular/angular/issues/41193#issuecomment-797554210

Packages must be compiled and ran using the exact same version of Angular; any other combination is not supported and likely to break in subtle ways.

So the @vendure/admin-ui/.. packages have been compiled with the compiler v11.2.4, but now you are trying to use those compiled files with compiler v11.2.5.

The solution is that I need to pin the dependencies of the ui-devkit package to exact patch versions which match those used by the admin-ui package. Currently they are listed as: https://github.com/vendure-ecommerce/vendure/blob/7cc7222f8241ddc5a2401faaed269a4a6f09f564/packages/ui-devkit/package.json#L39-L41

Wanztwurst commented 3 years ago

Can confirm, with compiler 11.2.4 it works.

"resolutions": {
    "@angular/cli": "11.2.4",
    "@angular/compiler-cli": "11.2.4",
    "@angular/compiler": "11.2.4"
},

What's the long term solution for this? Thank you for investigating!

Draykee commented 3 years ago

I think it's not the compiler version itself, but the forced resolution. When I put compiler version 11.2.3 into the resolutions

"resolutions": {
    "@angular/cli": "11.2.3",
    "@angular/compiler": "11.2.4",
    "@angular/compiler-cli": "11.2.4"
  }

it still works for me. Therefore I would say the solution is to have the same compiler version accross the projects.

michaelbromley commented 3 years ago

What's the long term solution for this?

I've implemented an automated check to ensure that Angular compiler versions match between the admin-ui & ui-devkit packages. This should prevent such errors being re-introduced in future.

martijnvdbrug commented 3 years ago

If you're using yarn workspaces or Lerna with yarn, don't forget to add the resolutions in your root package.json, not in child package.json's