Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(codegen): add serde helper function #4616

Merged
merged 2 commits into from
Apr 6, 2023
Merged

feat(codegen): add serde helper function #4616

merged 2 commits into from
Apr 6, 2023

Conversation

kuhe
Copy link
Contributor

@kuhe kuhe commented Apr 5, 2023

Issue

n/a

Description

This adds a serde helper called take. The anticipated usage is as follows:

current:

  {
    CreationDateTime:
      output.CreationDateTime != null
        ? __expectNonNull(__parseEpochTimestamp(__expectNumber(output.CreationDateTime)))
        : undefined,
    GlobalTableArn: __expectString(output.GlobalTableArn),
    GlobalTableName: __expectString(output.GlobalTableName),
    GlobalTableStatus: __expectString(output.GlobalTableStatus),
    ReplicationGroup:
      output.ReplicationGroup != null
        ? deserializeAws_json1_0ReplicaDescriptionList(output.ReplicationGroup, context)
        : undefined,
  }

new:

  take(output, {
    CreationDateTime: [, _ => __expectNonNull(__parseEpochTimestamp(__expectNumber(_)))],
    GlobalTableArn: __expectString,
    GlobalTableName: __expectString, // single fn is equivalent to [,fn,]
    GlobalTableStatus: __expectString,
    ReplicationGroup: [, _ => deserializeAws_json1_0ReplicaDescriptionList(_, context)]
  });

The use of tuples as instructions (essentially arguments) is for consistency in reading positional values, and brevity.

[filter function, mapping function, source key]

the default form can be reduced from [void 0, void 0, void 0] to [,,] and further to [], which is shorter than void 0.

Testing

existing and new unit tests

@kuhe kuhe added the smithy-codegen Changes regarding supporting smithy model. Will be merged to smithy-codegen branch label Apr 5, 2023
@kuhe kuhe requested review from a team as code owners April 5, 2023 17:00
@mtdowling
Copy link
Member

Would this account for the use of @jsonName, or is this only for protocols where the name always matches what's on the wire? Does this belong in smithy-typescript too?

@kuhe kuhe force-pushed the feat/serde branch 2 times, most recently from 39eaca9 to f425644 Compare April 5, 2023 17:07
@kuhe
Copy link
Contributor Author

kuhe commented Apr 5, 2023

Does this belong in smithy-typescript too?

Yes, this should migrate over with its module when that happens.

Would this account for the use of @jsonName, or is this only for protocols where the name always matches what's on the wire?

It should handle name differences between the source and target.

const source = {
  succinct: 'value 1',
  VERBOSE: 'value 2' 
};

const result = take(source, {
  succinct: [], // default instruction
  verbose: [_ => _ === 'value 2', _ => _ + ' append this', 'VERBOSE'] // verbose instruction
});

result == {
  succinct: 'value 1',
  verbose: 'value 2 append this'
}

The instruction is a triplet of [valueFilter, valueMapper, sourceKey] where the defaults are [nonNullish, passThrough, same key]

@kuhe kuhe force-pushed the feat/serde branch 2 times, most recently from bfae917 to adb2d2f Compare April 5, 2023 17:27
@kuhe
Copy link
Contributor Author

kuhe commented Apr 5, 2023

Applying this for example to Lambda's client, the npm pack package size without downlevel types goes from 151kb to 143kb.
The protocol file itself differs in wc as follows:

before/after

7407  24492 360472 dist-cjs/protocols/Aws_restJson1.js
6393  20720 317862 dist-cjs/protocols/Aws_restJson1.js

Example diff: #4620

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
smithy-codegen Changes regarding supporting smithy model. Will be merged to smithy-codegen branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants