Skip to content

fix: fixes issues in ql SELECT.from(...) when using typer classes#268

Merged
daogrady merged 7 commits into
cap-js:mainfrom
stockbal:fix/ql-for-typer-classes
Oct 24, 2024
Merged

fix: fixes issues in ql SELECT.from(...) when using typer classes#268
daogrady merged 7 commits into
cap-js:mainfrom
stockbal:fix/ql-for-typer-classes

Conversation

@stockbal

@stockbal stockbal commented Oct 5, 2024

Copy link
Copy Markdown
Contributor

This PR addresses the following issues

Sample schema

// model.cds
entity Books {
  key ID    : Integer;
      title : String;
}

Generated classes from cds-typer

class Book { }
class Books extends Array<Book> {}

Issues

1) No compiler issues, but both queries actually return type any instead of Book

let selectOne: Book

selectOne = await SELECT.from(Books, 42)
selectOne = await SELECT.from(Books, 42, (b: Book) => b.title)
  1. Produces typescript issues because the plural type is the assumed result of the queries
let selectOne: Book

selectOne = await SELECT.from(Book, 42)
selectOne = await SELECT.from(Book, 42, ["title"])
selectOne = await SELECT.from(Book, 42, (b: Book) => f.x)

@stockbal stockbal force-pushed the fix/ql-for-typer-classes branch from d9f3ec0 to 2fbb13b Compare October 7, 2024 11:59
@stockbal

stockbal commented Oct 7, 2024

Copy link
Copy Markdown
Contributor Author

Due to #201, Issue 1) no longer exists, and fix for issue 2) has been adjusted

let selectMany: Foos

// SINGULAR TESTS
selectOne = await SELECT.from(Foos.drafts, 42) // .drafts of plural still singular

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is actually an open question as of today. 🫠 But let's roll with it for now.

@daogrady daogrady left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for fixing this! 🙂

@daogrady daogrady enabled auto-merge (squash) October 24, 2024 06:21
@daogrady daogrady disabled auto-merge October 24, 2024 06:21
@daogrady daogrady enabled auto-merge (squash) October 24, 2024 06:24
@daogrady daogrady merged commit 9d83d3a into cap-js:main Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants