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

Dyngoose + SST doesnt work #710

Open
byeze opened this issue Aug 20, 2024 · 8 comments · May be fixed by #711
Open

Dyngoose + SST doesnt work #710

byeze opened this issue Aug 20, 2024 · 8 comments · May be fixed by #711
Labels
enhancement New feature or request

Comments

@byeze
Copy link

byeze commented Aug 20, 2024

Hello! I have a problem when using Dyngoose with SST. When using SST:

TypeError: Cannot read properties of undefined (reading 'get')\n    at getById

Model looks like this:

import type { PostLogSchema } from "@/schemas/postLog.schema";
import { Dyngoose } from "dyngoose";
import { Resource } from "sst";

@Dyngoose.$Table({ name: Resource.log.name })
export class LogModel extends Dyngoose.Table {
	@Dyngoose.Attribute.String()
	public id: string;

	@Dyngoose.Attribute.Date({ nowOnCreate: true })
	public createdAt: Date;

	@Dyngoose.$PrimaryKey("id")
	static readonly primaryKey: Dyngoose.Query.PrimaryKey<LogModel, string>

	@Dyngoose.$DocumentClient()
        // i setted the value of this prop because you can't `put` items, because `documentClient` is undefined
	static readonly documentClient: Dyngoose.DocumentClient<LogModel> =
		new Dyngoose.DocumentClient(LogModel);
}
@benhutchins
Copy link
Owner

I have not tested it with SST, however, I hope to use SST for my next project so I will certainly look into this.

@benhutchins benhutchins added the enhancement New feature or request label Aug 20, 2024
@byeze
Copy link
Author

byeze commented Aug 20, 2024

Thank you! :) In the meantime, I'll try to debug this

@byeze
Copy link
Author

byeze commented Aug 20, 2024

@benhutchins do you have any hint to track where documentClient, primaryKey, ... and those "defaults" values are set in a model? the problem occurs because those values are not set.

@benhutchins
Copy link
Owner

benhutchins commented Aug 20, 2024

There is no default primarKey object, it is defined in your table class and the object is created and property is assigned via the decorator. See the decorator here

export function PrimaryKey(primaryKey: string, sortKey?: string) {
which calls the setPrimaryKey on the Scehma class. The Schema class eventually calls definePrimaryKeyProperty when constructing and assigns an instance of the Query.PrimaryKey object to the Table based on whatever property is used on the Table, some people call it pk or similar instead of primarKey. It is actually constructed and assigned onto the Table when the @Dyngoose.$Table() decorator gets calls, which happens after all the decorator on the properties are processed. See
table.schema.definePrimaryKeyProperty()

The documentClient is part of Table class, it exists as a static property on the table. See

public static get documentClient(): DocumentClient<Table> {
which auto-creates a DocumentClient if requested but otherwise may not exist on the Table.

Your DocumentClient creation in your example code is actually incorrect, you don't need to have the code below at all.

	@Dyngoose.$DocumentClient()
        // i setted the value of this prop because you can't `put` items, because `documentClient` is undefined
	static readonly documentClient: Dyngoose.DocumentClient<LogModel> =
		new Dyngoose.DocumentClient(LogModel); // the actual object here will be overridden by the decorator, so instantiating the `new Dyngoose.DocumentClient` isn't necessary

However, if you want to define a custom property for the document client you just need to do:

  @Dyngoose.$DocumentClient()
  public static readonly documentClient: Dyngoose.DocumentClient<LogModel>

And the decorator will assign the property for you.

@byeze
Copy link
Author

byeze commented Aug 20, 2024

Excellent, thank you for the explanation! I think that the problem is when table.schema.definePrimaryKeyProperty() is called.
When removing documentClient from the table model, and only leaving the primary key, when trying to get an item using the PK, the following error appears:

Model used

import type { PostLogSchema } from "@/schemas/postLog.schema";
import * as Dyngoose from "dyngoose";
import { Resource } from "sst";

@Dyngoose.$Table({ name: Resource.log.name })
export class LogModel extends Dyngoose.Table {
  @Dyngoose.Attribute.String()
  public id: string;

  @Dyngoose.$PrimaryKey("id")
  static readonly primaryKey: Dyngoose.Query.PrimaryKey<LogModel, string>
}

Code executed

export async function getLogById(logId: string): Promise<LogModel> {
  return await LogModel.primaryKey.get({ id: logId });
}

Output

TypeError: Cannot read properties of undefined (reading 'get')\n    at getLogById

Tried logging the properties on LogModel (the table), but got undefined as the result. It's confusing why on deployments using serverless framework works, but when using SST this strange bug appears.

@benhutchins
Copy link
Owner

SST uses an ES modules, since Dyngoose doesn't have type: module in the package.json (actually defines commonjs), it runs into issues. Serverless is all commonjs (same with CDK, even though SST v1 used CDK).

I am sure this is where the issue is arising from. Hypothetically, you can import commonjs from an es module, but I've seen issues here before. Many projects today provide a different main export for cjs vs mjs. I imagine that is what Dyngoose needs to be updated with, since it uses TypeScript, the actual code changes to the typescript files shouldn't be dramatic but might need to define two separate tsconfigs and update the build process to build both.

I'll see if I can get some time this week to take a look into this.

@byeze
Copy link
Author

byeze commented Aug 20, 2024

Thank you! I'll try to fix this too.

benhutchins added a commit that referenced this issue Aug 29, 2024
Breaking Change: this changes the import paths for Dyngoose

Fixes #710
@benhutchins benhutchins linked a pull request Aug 29, 2024 that will close this issue
6 tasks
@benhutchins
Copy link
Owner

I'm not finished, however, I have a work in progress PR available #711 for review which adds esm support for Dyngoose. I will be performing more testing and will try to avoid a breaking change if possible (currently it is).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants