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.

Sign up for GitHub

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

Jump to bottom

Remove dependency on AWS-SDK #29

Closed
ruicsh opened this issue May 13, 2021 · 6 comments
Closed

Remove dependency on AWS-SDK #29

ruicsh opened this issue May 13, 2021 · 6 comments

Comments

@ruicsh
Copy link
Member

ruicsh commented May 13, 2021

We're only using the AWS-SDK to create a DynamoDBSet. Let's try and emulate it and drop the dependency
bilalq added a commit to bilalq/dynoexpr that referenced this issue Jul 4, 2022
@bilalq
feat(deps)!: Make aws-sdk a peer/dev dependency
4f4371b 
This partially addresses the problem raised in issue tuplo#29. It allows for
consumers to reliably load only their own AWS SDK version rather than
risking potentially bundling multiple copies in a build. It also makes
this package more Lambda function friendly, since the aws-sdk version is
vended by the runtime by default and does not need to be packaged in.

Note that this change does not make this package agnostic to the choice
between AWS SDK v2 and v3. The current implementation still relies on
imports that exist on v2 but not v3.
bilalq added a commit to bilalq/dynoexpr that referenced this issue Jul 4, 2022
@bilalq
refactor: avoid instantiating DocumentClient
ef7217b 
This refactor avoids instantiating a DynamoDB.DocumentClient object
(which in turn would instantiate an underlying DynamoDB service client)
by accessing the `createSet` method from the prototype directly.

The interface for `createSet` is:

```typescript
    createSet(list: number[]|string[]|DocumentClient.binaryType[], options?: DocumentClient.CreateSetOptions): DocumentClient.DynamoDbSet;
```

And the implementation is:

```typescript
  createSet: function(list, options) {
    options = options || {};
    return new DynamoDBSet(list, options);
  }
```

As we can see, there is no reliance on the `this` instance, so calling
the method from the prototype with an undefined `this` value should work
fine. It would have perhaps been preferred to utilize the `DynamoDBSet`
class directly, but that is marked with `@private` annotations within
the AWS SDK while the `createSet` method on DocumentClient is part of
the public API.

Aside from the removal of client instantiation, this change also scopes
imports to just the DynamoDB specific resources. This has been measured
to improve performance by several milliseconds in Lambda runtimes.

This somewhat relates to issue tuplo#29, but it doesn't really address it.
bilalq added a commit to bilalq/dynoexpr that referenced this issue Jul 4, 2022
@bilalq
feat(deps)!: make aws-sdk a peer/dev dependency
3eb652c 
This partially addresses the problem raised in issue tuplo#29. It allows for
consumers to reliably load only their own AWS SDK version rather than
risking potentially bundling multiple copies in a build. It also makes
this package more Lambda function friendly, since the aws-sdk version is
vended by the runtime by default and does not need to be packaged in.

Note that this change does not make this package agnostic to the choice
between AWS SDK v2 and v3. The current implementation still relies on
imports that exist on v2 but not v3.
This was referenced Jul 4, 2022
Merged
Merged
ruicsh pushed a commit that referenced this issue Jul 4, 2022
@bilalq @ruicsh
feat(deps)!: make aws-sdk a peer/dev dependency
49b74f1 
This partially addresses the problem raised in issue #29. It allows for
consumers to reliably load only their own AWS SDK version rather than
risking potentially bundling multiple copies in a build. It also makes
this package more Lambda function friendly, since the aws-sdk version is
vended by the runtime by default and does not need to be packaged in.

Note that this change does not make this package agnostic to the choice
between AWS SDK v2 and v3. The current implementation still relies on
imports that exist on v2 but not v3.
@ruicsh
Copy link
Member Author

ruicsh commented Jul 4, 2022

Now that the aws-sdk has moved to the peerDependencies, is there a need to write our own implementation of DynamoDB.Set? The goal has always been to make this lib dependency free, but it doesn't make sense to use it without a aws-sdk present. It's not very wise to have our own DynamoDB.Set, either. I think we can close this?

bilalq added a commit to bilalq/dynoexpr that referenced this issue Jul 4, 2022
@bilalq
feat: avoid instantiating DocumentClient
cee2239 
This refactor avoids instantiating a DynamoDB.DocumentClient object
(which in turn would instantiate an underlying DynamoDB service client)
by accessing the `createSet` method from the prototype directly.

The interface for `createSet` is:

```typescript
    createSet(list: number[]|string[]|DocumentClient.binaryType[], options?: DocumentClient.CreateSetOptions): DocumentClient.DynamoDbSet;
```

And the implementation is:

```typescript
  createSet: function(list, options) {
    options = options || {};
    return new DynamoDBSet(list, options);
  }
```

As we can see, there is no reliance on the `this` instance, so calling
the method from the prototype with an undefined `this` value should work
fine. It would have perhaps been preferred to utilize the `DynamoDBSet`
class directly, but that is marked with `@private` annotations within
the AWS SDK while the `createSet` method on DocumentClient is part of
the public API.

Aside from the removal of client instantiation, this change also scopes
imports to just the DynamoDB specific resources. This has been measured
to improve performance by several milliseconds in Lambda runtimes.

This somewhat relates to issue tuplo#29, but it doesn't really address it.
ruicsh pushed a commit that referenced this issue Jul 4, 2022
@bilalq @ruicsh
feat: avoid instantiating DocumentClient
7f89a18 
This refactor avoids instantiating a DynamoDB.DocumentClient object
(which in turn would instantiate an underlying DynamoDB service client)
by accessing the `createSet` method from the prototype directly.

The interface for `createSet` is:

```typescript
    createSet(list: number[]|string[]|DocumentClient.binaryType[], options?: DocumentClient.CreateSetOptions): DocumentClient.DynamoDbSet;
```

And the implementation is:

```typescript
  createSet: function(list, options) {
    options = options || {};
    return new DynamoDBSet(list, options);
  }
```

As we can see, there is no reliance on the `this` instance, so calling
the method from the prototype with an undefined `this` value should work
fine. It would have perhaps been preferred to utilize the `DynamoDBSet`
class directly, but that is marked with `@private` annotations within
the AWS SDK while the `createSet` method on DocumentClient is part of
the public API.

Aside from the removal of client instantiation, this change also scopes
imports to just the DynamoDB specific resources. This has been measured
to improve performance by several milliseconds in Lambda runtimes.

This somewhat relates to issue #29, but it doesn't really address it.
@bilalq
Copy link
Contributor

bilalq commented Jul 5, 2022

One gotcha is that this lib will currently only work with AWS SDK v2. It's not compatible with v3 because of different imports needed. There are four options, I think:

  1. Maintain two separate branches/versions of this package that target aws-sdk v2 and v3 separately. In that case, semver would be abandoned.
  2. Maintain two separate npm published packages that mostly share code but differ in terms of set creation for v2 and v3.
  3. Require the caller to pass in an optional DocumentClient reference that would only be required if the input contains a set. With that, the library would be able to defer to the caller.
  4. Have set creation work off Dynamic imports using require statements wrapped in try/catch blocks. Try importing v3, and if it's not available, import v2.

@bradens
Copy link

bradens commented Feb 28, 2023

I would love to see a v3 compatible version of this library. We personally use it quite heavily and are trying to remove deps on v2 aws-sdk versions.

@ruicsh
Copy link
Member Author

ruicsh commented Mar 1, 2023

ok, let me look into this. I favor option 3 on @bilalq post, what you guys think?

@bradens
Copy link

bradens commented Mar 3, 2023 •
edited

Yeah option 3 sounds good to me.

@ruicsh
Copy link
Member Author

ruicsh commented Mar 4, 2023

I just pushed v3 which no longer depends on AWS SDK.
If (and only if) you need to work with Sets, you provide it as a parameter like this:

import { DocumentClient } from "aws-sdk/clients/dynamodb";

const params = dynoexpr({
  DocumentClient,
  Update: {
    Color: new Set(['Orange', 'Purple'])
  },
})

Let me know if it works for you. All types are prefixed with I now, which makes it not backward compatible, but the code calls stay the same. If you're working with Sets and you don't provide the SDK it will throw an error.

@ruicsh ruicsh closed this as completed Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bilalq @bradens @ruicsh