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

[Spike] Explorer the way to resolve inclusion property #2152

Open
jannyHou opened this Issue Dec 13, 2018 · 1 comment

Comments

Projects
None yet
3 participants
@jannyHou
Copy link
Contributor

jannyHou commented Dec 13, 2018

Description / Steps to reproduce / Feature proposal

A follow-up story of the relation traverse spike #1952

The previous spike story turns out that we could improve our design of describing&defining model properties.

Copied the discussion and proposal from it:

Thanks for all the feedback!

I understand that the best outcome of a spike is a concrete solution, but I am afraid the inclusion story is more complicated than we expect, and it would be better to create separate stories to explorer different potential approaches.

Like what @raymondfeng points out, GraphQL's query system has two features we could borrow:

  • use resolver to describe a complicated object and calculate its value
  • use different query types to describe the filter of a model

And a overall design question to consider (from @bajtos ) :

How are we going to generate JSON Schema and OpenAPI schema for payloads containing both model properties (Customer's id, name, email) and data of related model (Customer's orders, addresses, etc.). At the moment, we are leveraging @Property metadata. If we choose a different approach for describing model+relations in TypeScript, then we also need a new way how to obtain JSON Schema for such new types.

Here is a proposal of the next steps:

  • Explorer the approach "Define relation property as a resolver"
    • The prototype would be:
         import { TodoList } from './TodoList';
         class Todo extends Entity {
            @inclusion()
            TodoList: ()=>TodoList
          }
  • Explorer the approach "Use a type wrapper to prevent the circular reference problem"
    • The prototype would be:
          export type Related<T> = T | undefined;
          class Todo extends Entity {
            // ...
            todoList: Related<TodoList>;
          }
          class TodoList extends Entity {
           // ...
            todos: Related<Todo>;
          }
  • Explorer "Generate decorator based filter types"
    • Inspired by the query/mutation type in GraphQL, I am trying to define different partial model types described by decorators
    • The prototype:
          // model.ts file
         class Todo extends Entity {
           @property({name: 'id', type: 'string'})
           id: string,
           @property({name: 'desc', type: 'string'})
           desc?: string
           @inclusion()
           TodoList: ()=>TodoList
         }
         // model.query.ts file
         type TodoMutation = Pick<Todo, a union of properties decorated by @property except id>
         type TodoQuery = TodoMutation & Inclusion<Todo>
  • Explorer the approach "Define multiple model classes for different purpose"
    • Prototype:
        class Customer extends Entity {
          // ...
        }
      
        class CustomerWithRelations extends Customer {
         orders?: Order[]
        }

@jannyHou jannyHou referenced this issue Dec 13, 2018

Closed

[Spike] How to do relation traversal #1952

0 of 3 tasks complete
@bajtos

This comment has been minimized.

Copy link
Member

bajtos commented Dec 14, 2018

use resolver to describe a complicated object and calculate its value
Explorer the approach "Define relation property as a resolver"

As I understand LB4 design, the Model class is describing shape of the data only, it does not deal with obtaining that data (fetching it from a database or resolving a model relation). It's the Repository that deals with resolving model relations and fetching related data.

We are following this design in HasMany/BelongsTo relation too. You can think of HasManyRepository.prototype.find and BelongsToAccessor as resolver functions.

I feel that your proposal proposal to move resolvers to model classes is violating this design constraint and we should investigate different approaches.

Re-posting the code sample from the problematic proposal:

   import { TodoList } from './TodoList';
   class Todo extends Entity {
      @inclusion()
      TodoList: ()=>TodoList
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment