chrisguttandin / dynamo-converters

A collection of converter functions to get good old JavaScript key/value objects into a DynamoDB friendly schema and back again.
MIT License
24 stars 3 forks source link

Configurable timestamps #79

Closed deini closed 2 years ago

deini commented 3 years ago

Would you be open to the idea of having some configuration options for deltaToExpression?

I'm mostly interested in being able to configure the attribute names for created and modified. Also their values to use Date().toISOString().

chrisguttandin commented 3 years ago

Yes, absolutely. I honestly thought about removing those attributes entirely from the library a couple of times already. With modern JavaScript it would be very little overhead to add them manually.

deltaToExpression({ ...delta, modified: Date.now() })

It would make it possible to not use those attributes at all or to use custom ones. The TypeScript types will help to prevent any spelling mistakes.

What do you think?

deini commented 3 years ago

Absolutely, if you are ok with the breaking change, that would be ideal IMO.

Speaking of breaking changes, what are your thoughts on changing the returned properties of deltaToExpression from camel case to pascal case to match dynamodb's client?

This is a bit 💅 but I can't currently do something like:

new UpdateItemCommand({
    ...deltaToExpression(delta),
    ..someOtherThings,
});
chrisguttandin commented 3 years ago

Yes, I think introducing a breaking change with a major version update is fine. It would also allow to modernize the type definitions.

I'm not sure if I understand your comment about the casing. Does v3 of the AWS SDK require a different JSON schema?

deini commented 3 years ago

I believe it's the same for v2 / v3 🤔

Unless I'm missing something, right now I have to do something like this:

const expression = deltaToExpression(delta);

new UpdateItemCommand({
  TableName: 'some-table',
  Key: marshall(...),
  ExpressionAttributeNames: expression.expressionAttributeNames,
  ExpressionAttributeValues: expression.expressionAttributeValues,
  UpdateExpression: expression.updateExpression,
});

If we change the casing we should be able to do:

const expression = deltaToExpression(delta);

new UpdateItemCommand({
  TableName: 'some-table',
  Key: marshall(...),
  ...expression
});
chrisguttandin commented 3 years ago

Ah, thanks for pointing that out. I'm so used to write ExpressionAttributeNames: expression.expressionAttributeNames that I didn't realize how odd that is.

Now that I think about it maybe the whole function should be named differently since it doesn't return an expression (or not only the expression). deltaToUpdateParams() might be more accurate.

deini commented 3 years ago

I agree, deltaToUpdateParams makes a bit more sense.

Now that @aws-sdk/util-dynamodb is available with marshall / unmarshall and DocumentClient doing marshalling / unmarshalling out of the box. I see more consumers reaching out for dynamo-converters for its deltaToUpdateParams (like we did 😄).

chrisguttandin commented 3 years ago

Oh yes, I almost forgot about marshal()/unmarshall(). When I first heard about it I was thinking that I could deprecate this library now but I then realized that it's not really type safe and for some reason the package requires you to use lib dom which I don't really want when developing server-side code.

I meanwhile created a PR with the changes we discussed above. #80 Could you please take a look?

You can install the updated version like this:

npm i git://github.com/chrisguttandin/dynamo-converters.git#remove-timestamp-and-rename-export-build