Removing properties from payload.properties or payload.traits in namespaced hooks doesn't propagate to downstream hooks

[JuroOravec]

Current behavior: If I have 2 plugins with e.g. a track hook, and one plugin defines a namespaced hook to run before the other plugin (track:pluginB in the example below), then when I try to remove a property from payload.property object in the upstream plugin, this change is not propagated to the downstream plugin (see example below).

This is unexpected behavior, as other changes seem to be propagated correctly.

Desired behavior: If I remove properties in the upstream plugin, the change should be propagated to the downstream plugin and the property should not be defined on the object there.

import { omitBy, isNil } from "lodash";

// plugin A
{
  name: "pluginA",
  "track:pluginB": ({ payload }) {
    // Returns payload with null or undefined values removed from
    // payload.properties
    return {
      ...payload,
      properties: {
        ...omitBy(payload.properties, isNil)
        test: true
    };
  }
  ...
};

// plugin B
{
  name: "pluginB",
  config: { payload },
  track:
    // The printed properties object includes `test: true`, so the payload
    // was clearly passed through pluginA["track:pluginB"], but the object
    // also still includes null and undefined values, although the `omitBy`
    // function removed them.
    console.log(payload.properties);
    return payload;
  ...,
};

[DavidWells]

I see what you mean. (IIRC) The payloads are merged https://github.com/DavidWells/analytics/blob/master/packages/analytics-core/src/middleware/plugins/engine.js#L233-L236 and it looks like those null keys still get passed through.

It seems like we could automatically strip null values here but it's tricky because a third-party analytics provider might require a specific key even if it's null.

Are the null values causing issues in your downstream third party tools?

[JuroOravec]

Hi David, thanks for the reply.

I don't have detailed insight into the codebase, so what's the purpose of merging the payloads? Is there a case where that's preferred over returning only the payload from the scoped method?

I agree that automatically stripping nulls wouldn't be good, as you said, some providers might need it.

And it wasn't causing issue in our case (we were using GA), but since we were developing open source project,