platform icon indicating copy to clipboard operation
platform copied to clipboard

`createEffect` implementation details are leaking

Open JeanMeche opened this issue 3 months ago • 1 comments

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

effects

Minimal reproduction of the bug/regression with instructions

This has already been reported by #3052, I understand the behavior and why It's not recommended to do that,

I think the implementation details of createEffect leak when the return returns a symbol.

myEffect = createEffect(() => {
  if(someCondition) {
     return EMPTY. 
  }
  ... 
});

or also

const obs:Observable<any> = new Observable();

const eff1 = createEffect(() => obs);
const eff2 = createEffect(() => obs);

Both codes throw Cannot redefine property: __@ngrx/effects_create__, which is basically an implementation detail of the createEffect function that would happen if defineProperty had the option configurable: true by default.

For the sake of compleness, this issue doesn't arise when the Zone.js legacy patch is loaded which changes the behavior of Object.defineProperty, see angular/angular/issues/37432

Expected behavior

The error should be more explicit / or it shouldn't error at all.

JeanMeche avatar Oct 04 '25 16:10 JeanMeche

createEffect is expected to return an object or function, but if a primitive (like a Symbol) is returned, it fails silently or throws a cryptic error. I think adding a guard could help clarify this behavior:

export function createEffect<
  Result extends EffectResult<unknown>,
  Source extends () => Result,
>(
  source: Source,
  config: EffectConfig = {}
): (Source | Result) & CreateEffectMetadata {
  const effect = config.functional ? source : source();
  const value: EffectConfig = {
    ...DEFAULT_EFFECT_CONFIG,
    ...config,
  };

  // 🚨 Guard against primitives like Symbol, string, number, boolean, null, undefined
  const isValidTarget =
    (typeof effect === 'object' && effect !== null) ||
    typeof effect === 'function';

  if (!isValidTarget) {
    throw new Error(
      `createEffect must return an object or function, but received: ${typeof effect}`
    );
  }

  Object.defineProperty(effect, CREATE_EFFECT_METADATA_KEY, {
    value,
    configurable: true, // Optional: allows safe redefinition
  });

  return effect as typeof effect & CreateEffectMetadata;
}

jdegand avatar Oct 04 '25 18:10 jdegand