ngneat / effects

🪄 A framework-agnostic RxJS effects implementation
https://www.netbasal.com
MIT License
63 stars 11 forks source link

Multiple effects with same class and function names #60

Closed nickathone closed 1 year ago

nickathone commented 1 year ago

Which @ngneat/effects-* package(s) are the source of the bug?

effects, effects-ng

Is this a regression?

Yes

Description

Previously using v2 of effects-ng I could register multiple effects classes with the same name, as well as properties with the same name.

However, having recently updated to v3, and changed to using provideEffectsManager and provideEffects it seems only the first effect class is registered and all others are ignored.

Hopefully, this illustrates the issue:

// Module 1
export class DataEffects {
    readonly fetch$ = createEffect((actions) =>
        actions.pipe(
            ofType(dataActions.fetch),
            switchMap(<call to API 1>),
       ),
    );
}

// Module 2
export class DataEffects {
    readonly fetch$ = createEffect((actions) =>
        actions.pipe(
            ofType(dataActions.fetch),
            switchMap(<call to API 2>),
       ),
    );
}

//----- THIS WORKED [v2]

// Module 1
EffectsNgModule.forFeature([DataEffects])

// Module 2
EffectsNgModule.forFeature([DataEffects])

//----- THIS DOES NOT WORK [v3] (only provideEffects to be registered first works)

// Module 1
provideEffects(DataEffects)

// Module 2
provideEffects(DataEffects)

Please provide a link to a minimal reproduction of the bug

No response

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in

@angular/*: 16.0.4
@ngneat/effects-ng: 3.1.2

Anything else?

No response

Do you want to create a pull request?

No

nickathone commented 1 year ago

I created a StackBlitz replicating the issue: https://stackblitz.com/edit/stackblitz-starters-wg1fa5?file=src%2Fmodule.ts

NetanelBasal commented 1 year ago

@EricPoul

EricPoul commented 1 year ago

@nickathone please update to the latest version to check if it works

nickathone commented 1 year ago

@EricPoul I have tested your change via a local build of the master branch. Seems to be working as expected now.

When will this be pushed to npm?

Thanks 👍

EricPoul commented 1 year ago

@NetanelBasal

NetanelBasal commented 1 year ago

Done