map fields are always initialized to {} even when not required and not defined

[asquithea]

Describe the bug
Given an entity like this:

new Entity(
  {
    model: {
      entity: "foo",
      version: "1",
      service: "service"
    },
    attributes: {
      tenantName: {
        type: "string",
        required: true
      },
      configuration: {
        type: "map",
        required: false,
        properties: {
          alpha: {
            type: "any",
          },
          beta: {
            type: "any"
          }
        }
      },
    },
    indexes: {
      byTenantName: {
        pk: {
          field: "pk",
          composite: ["tenantName"]
        },
      },
    }
  },
  { table }
);

and initialization code like this:

    const item = await this.entity
      .create({
        tenantName,
      })
      .go();

I had expected only the tenantName field to be initialized.
However, configuration is initialized to an empty map:

{
  "pk": {
    "S": "$foo#tenantName_ttt"
  },
  "configuration": {
    "M": {}
  },
  "tenantName": {
    "S": "ttt"
  },
  "__edb_e__": {
    "S": "foo"
  },
  "__edb_v__": {
    "S": "1"
  }
}

I was surprised by this behaviour, and it broke a design that assumed the presence or otherwise of this field had meaning (something I can probably work around).

ElectroDB Version
1.11.1

ElectroDB Playground Link
Playground Link

Entity/Service Definitions
See above.

Expected behavior
I expected the field (e.g. configuration) to be absent until it was initialized.

Errors
n/a

Additional context
Speculation:

Haven't tried to seriously debug, but at first glance the _makeSet method for the MapAttribute in schema.js looks suspect in the way it creates an empty data object which is always returned.

_makeSet(set, properties) {
	this._checkGetSet(set, "set");
	const setter = set || ((attr) => attr);
	return (values, siblings) => {
		const data = {};
		if (values === undefined) {
			return setter(data, siblings);
		}
		for (const name of Object.keys(properties.attributes)) {
                  // ...
		}
		return setter(data, siblings);
	}
}

[tywalch]

Two items:

1. This is value is added deliberately, could you describe your use case?
2. Your example uses “any” attributes with a map which is currently not supported and could likely result in issues. I had thought the typing and validation did not allow this but I will also need to look into that.

[asquithea]

Sure. In this project, the presence of the attribute is treated like a typical nullable variable:

• if it's there, do the thing
• if it's not there, don't do the thing

Concretely, the attributes represent some semi-structured configuration to be synchronized with a loosely-coupled system.

• configuration - currently saved configuration, initially missing
• requestedConfiguration - desired state of the other system; null/missing = undeploy wanted
• activeConfiguration - reported state of the other system; null/missing = undeploy achieved

So the presence is used to indicate whether the deployment should be active.
I have some other examples, like an attribute that either contains a filled out report object, or no report object at all.

Regarding any, each config map contains both some well-structured data, but also some arbitrary properties to be passed along. Kinda like this:

interface DeploymentConfiguration<P> {
  authentication: DeploymentAuthentication;  // strongly typed and required
  systemProperties: P | undefined;  // arbitrary system-specific properties
}

So in summary there is a well-defined object containing an island of arbitrary data. Let's say authentication is required so DeploymentConfiguration object can't be partially initialized - either the whole thing is present, or it's not.

This seems like it should be representable in the schema:

attributes: {
  configuration: {
    type: "map",
    required: false,
    properties: {
      authentication: {
        type: "map",
        properties: authProperties,
        required: true,
      },
      systemProperties: {
        type: "any",
        required: false,
      },
    },
  },
}

It generates the correct type signature, with the configuration attribute being defn | undefined and the authentication property being non-nullable, but then contradicts that type signature by initializing without authentication defined - something that would not be accepted if I called .set(...) with the same argument.

Hopefully that's clear enough.

Apologies if I'm not seeing the wood for the trees. I've done a couple of projects in straight DynamoDB so far, but there are so many gotchas (e.g. many reserved words) that only really emerge during testing, whereas your library deals with a lot of that up front.

I did see your note about previous behaviour of get returning an empty object, so it's possible I've just stumbled into my first example of something you've used widely as a convention maybe prior to adding Typescript types, so maybe not changable.

One possible workaround for me would be something like:

configuration: {
  type: "map",
  properties: configProperties,
  set: (a) => (_.isEmpty(a) ? undefined : a),

But it looks like this should be the default behaviour, otherwise the generated types don't match the initial value.

I can probably also get away with just making the entire configuration (and friends) attributes any since I don't need to be able to query on the contents, but obviously that may not be the case for some use cases.

[DASPRiD]

@tywalch We ran into the same issue. Is there a reason why this issue was closed? We'd like to have a map object (with required fields in it if it is present) on our entity, but be able to omit the object. Right now, the empty object breaks the Typescript typings.

[RobbeCl]

We are having the same issue. We have a few different user types and we store them all in the same User schema. A normal user has an Address map property, but an admin does not. Now the admin ends up with an empty object in the attribute Address and we like it to be undefined.

[tywalch]

Thank you three for bring this up, I will have a fix in this week 👍

[tywalch]

This was addressed with the following PR: #168

I'll close this issue for now, but if anything comes up feel free to reopen and I'll make it a priority to address 👍