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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Cannot read properties of undefined (reading 'name') at isClassInstance" when importing effects from a library #3972

Closed
1 of 2 tasks
mnlttt opened this issue Jul 27, 2023 · 12 comments · Fixed by #3984
Closed
1 of 2 tasks

Comments

@mnlttt
Copy link

mnlttt commented Jul 27, 2023

Which @ngrx/* package(s) are the source of the bug?

effects

Minimal reproduction of the bug/regression with instructions

  1. Import into your app a library that exports a provider with effects;
  2. Run your app.
  3. ERROR TypeError: Cannot read properties of undefined (reading 'name')
    at isClassInstance (ngrx-effects.mjs:135:29)

Minimal reproduction of the bug/regression with instructions

// node_modules/@ngrx/effects/fesm2022/ngrx-effects.mjs
function isClassInstance(obj) {
-     return (obj.constructor.name !== 'Object' && obj.constructor.name !== 'Function');
+     return (obj.constructor && obj.constructor.name !== 'Object' && obj.constructor.name !== 'Function');
}

function mergeEffects(sourceInstance, globalErrorHandler, effectsErrorHandler) {
-    const sourceName = getSourceForInstance(sourceInstance).constructor.name;
+    const sourceName = getSourceForInstance(sourceInstance)
+      ? getSourceForInstance(sourceInstance).constructor.name
+      : null;

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s)

NgRx: ^16.1.0
Angular: ^16.1.7
Node: 18.17.0

Other information

No response

I would be willing to submit a PR to fix this issue

  • Yes
  • No
@markostanimirovic
Copy link
Member

Hi @mnlttt,

I couldn't reproduce this behavior. Can you provide reproduction via Stackblitz playground or GitHub repo?

@mnlttt
Copy link
Author

mnlttt commented Jul 27, 2023

@markostanimirovic I don't have a public published library I'm sorry, but I can give you more details about how to reproduce it:

Using Nx I built a buildable and publishable library where I had this:

// my-effects.effects.ts
export const myEffect = createEffect(
  [...],
  { functional: true },
);
import * as MyEffects from './my-effects.effects';

export const myStoreProvider: () => EnvironmentProviders = () =>
  makeEnvironmentProviders([
    provideEffects(MyEffects),
  ]);

export const myProvider = (): EnvironmentProviders =>
  makeEnvironmentProviders([
    myStoreProvider(),
  ]);

Then, in my App, I imported myProvider into app's providers.

providers: [
  myProvider(),
],

BUT, if I remove myStoreProvider() from myProvider, and I import myStoreProvider() from my app, it works.

export const myProvider = (): EnvironmentProviders =>
  makeEnvironmentProviders([
  ]);
providers: [
  myProvider(),
  myStoreProvider(),
],

EDIT
photo_5336832530315332556_y
photo_5336832530315332568_y

The thing that must happen to get the error is to build the library and install it (npm pack works too).

@markostanimirovic
Copy link
Member

Thanks for more info! But as we mentioned in the bug report template, reproduction via GitHub repo/StackBlitz playground is required, especially for more complex scenarios.

Feel free to create an Nx repo that reproduces mentioned behavior, share the link here, and we'll take a look. Maybe the problem is not related to NgRx at all. It's weird that an object with effects (or any other object) doesn't have a constructor.

@mnlttt
Copy link
Author

mnlttt commented Jul 28, 2023

While I'll find the time to create a public repo, do you have any guesses why when importing effects directly from the app they're an esModule and everything works as expected
CleanShot 2023-07-28 at 15 39 11@2x
but when imported from the installed library they're a plain object and it throws the error?
CleanShot 2023-07-28 at 15 42 19@2x

I tested on a freshly created app too and it's the same.

@mnlttt
Copy link
Author

mnlttt commented Jul 28, 2023

@markostanimirovic there you go https://github.com/mnlttt/ngrx-app

It already conveniently has the library built, in the root. The lib is this https://github.com/mnlttt/ngrx-lib

I tried running the app on StackBlitz but I get this error:

NX Missing Platform Dependency
The Nx CLI could not find or load the native binary for your supported platform (linux-x64)

https://stackblitz.com/github/mnlttt/ngrx-app/

@mnlttt
Copy link
Author

mnlttt commented Jul 29, 2023

Published the package on npmjs and made the app without Nx.

https://stackblitz.com/github/mnlttt/ngrx-app-vanilla?file=src%2Fmain.ts

IMPORTANT: add import 'zone.js'; in src/main.ts.

@markostanimirovic
Copy link
Member

Thanks @mnlttt! I'll take a look.

@avine
Copy link

avine commented Aug 2, 2023

I have the same issue.

  • My app is entirely standalone
  • I have registered the effects using provideEffects function
  • I have tried class based and functional based effect

After building an Angular library that uses effects, i have the same error in the console at line ngrx-effects.mjs:134.

The error is that in the function isClassInstance, the given obj parameter (something like { myEffect$: () => {…} }) is an object that strangely does not have a constructor property. So the statement obj.constructor.name fails!

ERROR TypeError: Cannot read properties of undefined (reading 'name')
    at isClassInstance (ngrx-effects.mjs:134:29)
    at ngrx-effects.mjs:322:55
    at groupBy.js:25:29
    at OperatorSubscriber._next (OperatorSubscriber.js:13:21)
    at OperatorSubscriber.next (Subscriber.js:31:18)
    at Subject.js:34:30
    at errorContext (errorContext.js:19:9)
    at EffectSources.next (Subject.js:27:21)
    at EffectSources.addEffects (ngrx-effects.mjs:316:14)
    at useValue (ngrx-effects.mjs:676:35)

On the other hand, when running the library from source code (without building), everything works fine.

@mnlttt
Copy link
Author

mnlttt commented Aug 2, 2023

Avoiding the import/export * as syntax solves the problem, but I think we all could agree that this solution isn’t the best considering that now we have to import every single effect one by one.

@markostanimirovic
Copy link
Member

Agree 👍

Btw, the issue exists because import * as usersEffects is transpiled to:

const usersEffects = Object.freeze({
  __proto__: null,
  // ... functional effects
})

@avine
Copy link

avine commented Aug 2, 2023

As pointed above #3972 (comment), avoiding the export * as syntax solves the problem.

Here is my current solution:

import { inject } from '@angular/core';
import { Actions, createEffect, FunctionalEffect } from '@ngrx/effects';

// Do not export each effect
const myEffect$ = createEffect(
  (actions$ = inject(Actions)) => {
    return actions$.pipe(/* ... */);
  },
  { functional: true }
);

// Just add this
export const myFeatureEffects: Record<string, FunctionalEffect> = { myEffect$ };

Then, use your effects as usual:

import { myFeatureEffects } from './my-feature.effects.ts';

provideEffects(myFeatureEffects);

@marcindz88
Copy link

marcindz88 commented May 14, 2024

For those who still have to use version below 16.2.0 for some reason, you can use this as a quick fix:

import * as MyEffects from './my-effects.effects';

export const myStoreProvider: () => EnvironmentProviders = () =>
  makeEnvironmentProviders([
    provideEffects({ ...MyEffects }),
  ]);

Spread operator creates a new object which does have a proper constructor and problem is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants