[SUGGESTION] Use types instead of classes/functions for generation of cds entities/types/aspects

[stockbal]

Description

The currently used approach with classes and functions seems overcomplicated and is also kind of hard to read. In addition it feels weird to map class definitions to objects (cds.entities). The direct usage of cds.entities on root level of the generated modules also require dynamic imports to avoid runtime issues (in jest this is still an experimental feature).

Suggested Solution

• Use type instead of class
• The types for entities (singular/plural) and for types/aspects contain only the properties like declared in cds
• aspect/inheritence chains are realized with type intersections

  entity Books : cuid, managed {}
  

  type Book {} & __.cuid & __.managed
  

• each namespace file (index.js/index.ts) will get a new function export called entities which will return a typed version of cds.entities(<namespace>). This return type Entitites of that function, will contain extended versions of the plain types that will also contain the properties like actions or drafts. The elements of that property would what the current class definitions provide.

Example for index.ts

import * as _ from './..';
import * as _my_bookshop from './../my/bookshop';
import * as __ from './../_';
export default { name: 'CatalogService' }

// NOTE: inline enums will be placed under the namespace of the declaring entity
export namespace Book {
  // enum
  export const type = {
    Ebook: "Ebook",
    Hardcover: "Hardcover",
    Paperback: "Paperback",
  } as const;
  export type type = "Ebook" | "Hardcover" | "Paperback"
}

export type Book = {
  title?: string | null
  genre?: _my_bookshop.Genre | null
  type?: Book.type | null
  stock?: number | null
  author?: __.Association.to<_my_bookshop.Author> | null
  author_ID?: string | null
  publishers?: __.Composition.of.many<Books2Publishers>
} & _.cuid & _.managed & _my_bookshop.Generic
export type Books = Array<Book> & {$count?: number}

export type Books2Publisher = {
  book?: __.Association.to<_my_bookshop.Book> | null
  book_ID?: string | null
  publisher?: __.Association.to<_my_bookshop.Publisher> | null
  publisher_ID?: string | null
} & _.cuid
export type Books2Publishers = Array<Books2Publisher> & {$count?: number}

// type for cds.entities of namespace CatalogService
type Entities = {
  Book: __.Constructable<Book> & __.singular & __.withName & {
    actions: {
      order: { (quantity: number | null): any, __parameters: {quantity: number | null}, __returns: any, kind: 'action'}
    }
    drafts: __.Constructable<__.DraftEntity<Book>>
  }
  Books: __.ArrayConstructable<Book> & __.withName & {
    drafts: __.ArrayConstructable<__.DraftEntity<Book>>
  }
  Books2Publisher: __.Constructable<Books2Publisher> & __.singular & __.withName & {
    drafts: __.Constructable<__.DraftEntity<Books2Publisher>>
  }
  Books2Publishers: __.ArrayConstructable<Books2Publisher> & __.withName & {
    drafts: __.ArrayConstructable<__.DraftEntity<Books2Publisher>>
  }
}
/**
 * @returns {Entities} entities of namespace "CatalogService"
 */
export declare function entities(): Entities

Example of index.js

const cds = require('@sap/cds')
module.exports = { name: 'CatalogService' }
let _entities
module.exports.entities = function() {
  if (_entities) return _entities
  const csn = cds.entities('CatalogService')
  _entities = {
    Book: { is_singular: true, __proto__: csn.Books },
    Books: csn.Books,
    Books2Publisher: { is_singular: true, __proto__: csn.Books2Publishers },
    Books2Publishers: csn.Books2Publishers,
  }
  return _entities
}
// enums
module.exports.Book = {}
module.exports.Book.type ??= { Ebook: "Ebook", Hardcover: "Hardcover", Paperback: "Paperback" }

Sample Service implementation

import { ApplicationService } from "@sap/cds";
import * as cats from "#cds-models/CatalogService";
import { entities } from "#cds-models/my/bookshop";

export default class CatService extends ApplicationService {
  override async init() {
    const { Books } = cats.entities();

    this.before("UPDATE", Books.drafts, async (req) => {
      const { Authors } = entities();
      const author = await SELECT.one.from(Authors).columns("name");

      if (!author) req.reject(400, "Author not found");
    });
    return super.init();
  }
}

I know this change would break a lot of projects that are currently using cds-typer (including some I am personally involved in 😅) but I think it poses some benefits and should at least be discussed.

Benefits

• leaner and more readable types
• index.js and index.ts move closer together
• get rid of dynamic imports

This approach is currently implemented and functional on the following branch https://github.com/stockbal/cds-typer/tree/refactor/replace-classes-with-types

P.S.: This branch also contains a surefire way to only provide the drafts property in entities when it's truly there during runtime.

Tell me what you think about this new approach 😎

Alternatives

No response

Additional Context

No response

[daogrady]

Hi Ludwig,

thanks for the detailed description and all the work you have put into this suggestion!

The main reason why we went with classes over type-definitions or interfaces is that the generated types have to be usable in JavaScript projects. As classes have runtime equivalents, the LSP will give appropriate suggestions when you are importing class definitions from a .ts file into a .js file. As types and interfaces are completely erased at runtime, the LSP will not suggest them at all.

You can verify this with the following setup:

// types.ts
export class Foo {
    foo: number;
}
export type Bar = {bar: number}
export interface Baz {
    baz: number;
}

// index.js
const types = require('./types')

types. // trigger intellisense and observe how only Foo is suggested

While you can still import types and use them as part of JSDoc when you know they exist, this falls a bit short in some regards we wanted to help the user with, such as getting a list of entities to destructure the import.

So while I fully agree that this would be a good design choice if we would only cater to TypeScript projects, our responsibility towards our JavaScript users restricts us to classes, I am afraid.

Best, Daniel

[stockbal]

Hi Daniel,

thanks for the quick response. I have to disagree with you as my solution is also fully usable in JavaScript projects.

const { ApplicationService } = require("@sap/cds");
const cats = require("#cds-models/CatalogService");
const { entities } = require("#cds-models/my/bookshop");

module.exports = class CatService extends ApplicationService {
  async init() {
    const { Books, Book } = cats.entities();

    this.before("UPDATE", Books.drafts, async (req) => {
      const { Authors } = entities();
      const author = await SELECT.one.from(Authors).columns("name");

      if (!author) req.reject(400, "Author not found");
    });
    return super.init();
  }
};

When requiring an entity for CQL or handler registration you would only go via the new entities function. Let's consider a small repository class to wrap access to the Books entity

const { Book, entities } = require("#cds-models/my/bookshop");

class BookRepo {
  /**
   * @param {Book.type} type
   * @returns
   */
  async findBooksByType(type) {
    const { Books } = entities();
    return SELECT.from(Books).where({ type: type });
  }
}
module.exports = { BookRepo };

When accessing this class you also have full intellisense that the return type of findBooksByType is indeed Book[] without the need for using JSDoc type imports

[stockbal]

And types would actually also work without the entities function, but then we are again stuck with the dynamic imports, which is the main part I wanted to get rid of 😊.

index.ts

type TBook = {
  title?: string | null
} & __.cuid & __.managed

type TBooks = Array<TBook> & {$count?: number}

export declare const Book: __.Constructable<TBook> & { drafts: __.Constructable<__.DraftEntity<TBook>> }
export declare const Books: __.ArrayConstructable<TBook> & { drafts: __.ArrayConstructable<__.DraftEntity<TBook>> }

index.js

const cds = require('@sap/cds')
const csn = cds.entities('CatalogService')

module.exports.Book = { is_singular: true, __proto__: csn.Books  }
module.exports.Books = csn.Books

[daogrady]

Hi Ludwig,

thanks for the follow-up! I will gladly give your approach a more thorough look. But if you are actually after the dynamic imports, then this may be an x-y-problem to begin with. We have this shortcoming on the radar as we are not exactly happy with bereaving our users of plain old top-level imports. There may be an easier solution by slightly adjusting the way cds.entities(...) works (which is the reason for having to resort to dynamic imports). Would being able to use regular imports without changing the way cds-typer generates its artifacts help in your case?

Best, Daniel

[stockbal]

Hi Daniel,

yes that that would help. I mostly rebuild the generated classes into types to play well together with the entities function I created. It didn't make sense to have the static properties (likedrafts and actions) anymore, as I had to add those to the Entities type members.

But if you can somehow swing it that we can use standard imports everywhere then I can live with classes 😊.

Now I am curious how you could fix cds.entities(...) to achieve that 😎.

Best regards, Ludwig

[daogrady]

Hi Ludwig,

Now I am curious how you could fix cds.entities(...) to achieve that 😎.

the idea is to try to move loading the services a bit back in time during the spinup to make sure calling cds.entities(...) does not cause any issues by then. But this is an idea we still have to explore and see if it is a rabbit hole. I will bring this up during our next types-meeting and get back to you about this.

Best, Daniel

[stockbal]

Hi Daniel,

hm, not sure if that helps everywhere - or I understand it wrong.
I actually didn't have any issues in the service implementation files with regular imports of @cds-models. The problems start mostly in jest tests, because the imports are resolved before the server is started via cds.test(...).

I just thought of a different solution that would work without the need to modify anything in the cds core → using Proxy.

index.js

// This is an automatically generated file. Please do not change its contents manually!
const cds = require("@sap/cds");

module.exports = { name: "CatalogService" };

function createEntityProxy(
  fq,
  name,
  { target, customProps } = { target: {}, customProps: [] }
) {
  return new Proxy(target ?? {}, {
    get: function (target, prop) {
      // we already know the name so we skip the cds.entities proxy access
      if (prop === "name") return fq;
      // mostly is_singular is accessed
      if (target.hasOwnProperty(prop)) return target[prop];
      // inline enums have to be caught here, as they do not exist on the entity
      if (customProps.length > 0 && customProps.includes(prop)) return target[prop];
      // last but not least we pass the property access to cds.entities
      if (cds.entities) return cds.entities("CatalogService")[name][prop];
      throw new Error(`Property ${prop} does not exist on entity '${fq}' or cds.entities is not yet defined`);
    },
    set: function (target, prop, newVal) {
      target[prop] = newVal;
      return true;
    },
  });
}

module.exports.Books = createEntityProxy("CatalogService.Books", "Books" });
module.exports.Book = createEntityProxy("CatalogService.Books", "Books", { target: { is_singular: true }, customProps: ["type"] });
module.exports.Books2Publishers = createEntityProxy("CatalogService.Books2Publishers", "Books2Publishers");
module.exports.Books2Publisher = createEntityProxy("CatalogService.Books2Publishers", "Books2Publishers", { target: { is_singular: true } });

// enums
module.exports.Book.type ??= {
  Ebook: "Ebook",
  Hardcover: "Hardcover",
  Paperback: "Paperback",
};

BR, Ludwig

[stockbal]

Hi Daniel,

I created PR #281 for the proxy topic. Let's move any discussion about that to the PR.

BR, Ludwig

[stockbal]

Will close this, as the main problem is solved with the new proxy feature